Botany Rework Part 3: Plants as Entities #31941

drakewill-CRL · 2024-09-08T02:50:08Z

About the PR

This PR creates plants as their own Entities, instead of a bunch of properties on a PlantHolder. SeedData gets moved to the PlantComponent.

This is part 3 of my Botany refactor writeup. Part 2 covers growth components. This is in draft state while I work on this logic for early feedback. While I work on moving and testing code, I'm only going to test on wheat stalks. I'll move everything to the final setup in YAML when I get there.

Why / Balance

As mentioned in the core writeup and other PRs, botany can't really get any major changes because the backend system is mostly handled in one big function and adding anything to that only makes it more difficult to clean up. This strips the plant-specific part out so PlantHolder can just update its lights and resources and the Plant can handle its tasks. Between this, growth components, and the mutation rework, the Botany code will be in a much better state for future maintenance and additions.

This should have no bearings on game balance, because this is just a refactor. A few bugs might be fixed or created in the process of porting code rather than porting poor/broken behavior.

Technical details

Plants are now entities. They are connected to their PlantHolder, and they are spawned when seeds are planted and deleted when they're harvested (if single harvest) or when the shovel is used to pull them out. SeedData is moved to the PlantComponent now.

All the existing plant logic was moved into a single Update() on PlantSystem. Breaking that out to separate components and systems is done in Part 2. It may be too granular to break each stat into its own component/system pair. Merging these should mostly require operating on Plants instead of PlantHolders.

Plants still need a PlantHolder. PlantHolder still has the water/nutrients the plant needs so the plant still need to have a connection to that. I have helper functions that get the Plant, PlantHolder, and SeedData all at once because of how often all 3 get referenced. The 2 entities keep a reference to the other, and blank it out when the plant is removed from the tray.

Chemicals are still applied to and run by the PlantHolder. The code that increases the water/nutrient level in the PlantHolder is a PlantEffect, so those all need to be run on the PlantHolder and then determine if the plant needs to be involved on each. Splitting this up to make more logical sense is a future enhancement.

Plants should grow identically to before, almost entirely unnoticeable to players. As entities, you can see them instead of the tray and view them in the right-click menu. That's visibly different. You also need to apply different items to different entities (EX: clippers must be used on the plant, but spades and chemicals need to be used on the tray itself).

The Slippery mutation no longer applies to the plant itself. It continues to apply to the produce harvested. I could not work out a way to make the plant entity both correctly fixed to the PlantHolder and get it to register the stepOn collision correctly. Oddly, it could handle a StepOFF trigger correctly but that happens once you've completely walked off the entity and it didn't feel right when everything else slips you when you step on it. Making the plant itself not slippery is the simplest, lowest-impact change to be made for this.

Additional changes:

SplatPrototype, IdealLight, and LightTolerance were removed as a cleanup task, because they're unused stats, and now's as good a time as any to finish getting rid of unused code.

Media

Requirements

I have read and I am following the Pull Request Guidelines. I understand that not doing so may get my pr closed at maintainer’s discretion
I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

PlantHolder is pretty dramatically different.

Changelog

🆑

tweak: Botany plants are now their own entity. You may need to click on the tray or the plant to do what you want, depending on what you're holding.

mirrorcult · 2024-09-08T02:58:59Z

🙏

nymhole · 2024-09-12T18:31:26Z

this would also fix every sentience mutation being either hydroponics tray or soil? and it would kill the sentience with the plant?

drakewill-CRL · 2024-09-12T18:46:04Z

this would also fix every sentience mutation being either hydroponics tray or soil?

Yes, This would let the plant itself be sentient.

and it would kill the sentience with the plant?

As written right now, without merging any of the other pieces smartly, this kills the sentience when the plant is destroyed (on shovel use to uproot, or on harvest if its a single-harvest crop). Whether sentient plants should stop aging when possessed, or get an extended lifespan, or just be fully at the mercy of a botanist's skills and patience is an unasked question. I would like the refactors to avoid changing balance or features as much as possible, so I expect that this would go in with "dead plants can talk, talking plants die on harvest" as known side effects and a discussion on how/if to change sentient plants occurs then.

github-actions · 2024-09-15T03:21:23Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

DTSpawn · 2024-11-02T12:42:02Z

Please keep idealLight. I'm using it on my fork on some plants (they need to glow in dark places)

github-actions · 2024-11-04T01:05:00Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…e-station-14 into plantasentity

Qwazori · 2025-01-07T23:53:11Z

Still in progress?

drakewill-CRL · 2025-01-09T15:10:59Z

Still in progress?

Yes. I haven't made a big deal of trying to push my changes through. The maintainers have had a pretty big pile of nonsense to deal with since I started this, and I don't want to add any additional pressure on them for a refactor PR.

slarticodefast · 2025-01-09T16:22:24Z

Yeah, I'm sorry for the wait, things have been really busy recently. I still got the review for this on my ToDo list and hope I can do it soon. But for a review this size I need at least half a day of free time.

MilonPL

Okay some thoughts: I really don't like the GetEverything method and how we're overwritting the PlantComponent in a lot of cases, it seems really pointless. You could really play around with the Entity and Entity<T?> patterns for signatures, some of this seems like it could be replaced with queries, especially stuff called from inside of Update(), and then passed on to other methods (which I think would be a better way of getting that data)

MilonPL · 2025-01-31T14:58:36Z

Content.Client/Botany/Components/PlantVisualsComponent.cs

+namespace Content.Client.Botany.Components;
+
+[RegisterComponent]
+public sealed partial class PlantVisualsComponent : Component


This doesn't need a body declared.

MilonPL · 2025-01-31T14:59:24Z

Content.Client/Botany/PlantVisualizerSystem.cs

+        SubscribeLocalEvent<PlantVisualsComponent, ComponentInit>(OnComponentInit);
+    }
+
+    private void OnComponentInit(EntityUid uid, PlantVisualsComponent component, ComponentInit args)


Entity<T> ent, ref event args pattern.

MilonPL · 2025-01-31T15:00:08Z

Content.Server/Botany/Components/PlantComponent.cs

+[Access(typeof(BotanySystem), typeof(PlantSystem), typeof(PlantHolderSystem), typeof(EntityEffect))]
+public sealed partial class PlantComponent : Component
+{
+    [DataField(customTypeSerializer: typeof(TimeOffsetSerializer))]


Add the AutoPausedField attribute.

MilonPL · 2025-01-31T15:00:14Z

Content.Server/Botany/Components/PlantComponent.cs

+    [DataField]
+    public int LastProduce;
+
+    [DataField(customTypeSerializer: typeof(TimeOffsetSerializer))]


Add the AutoPausedField attribute.

MilonPL · 2025-01-31T15:01:24Z

Content.Server/Botany/Components/PlantComponent.cs

+    /// The time between growth ticks. 
+    /// </summary>
+    [DataField]
+    public TimeSpan CycleDelay = TimeSpan.FromSeconds(15f);


MilonPL · 2025-01-31T15:24:35Z

Content.Server/Botany/Systems/PlantSystem.cs

+    /// <summary>
+    /// Cease the growth and harvesting of a plant through the conclusion of biological processes.
+    /// </summary>
+    public void Die(EntityUid uid, PlantComponent? plant = null)


Overwritten.

MilonPL · 2025-01-31T15:24:42Z

Content.Server/Botany/Systems/PlantSystem.cs

+    /// </summary>
+    public void Die(EntityUid uid, PlantComponent? plant = null)
+    {
+        if (!GetEverything(uid, out plant, out var seed, out var holder) || plant == null)


seed can be discarded.

MilonPL · 2025-01-31T15:27:28Z

Content.Server/Botany/Systems/PlantSystem.cs

+    /// </summary>
+    public void UpdateSprite(EntityUid uid, PlantComponent? component = null)
+    {
+        GetEverything(uid, out var plant, out _, out _);


Why do we need two separate plant components?

MilonPL · 2025-01-31T15:27:59Z

Content.Server/Botany/SeedPrototype.cs

    /// </summary>
-    [DataField("packetPrototype", customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
+    [DataField(customTypeSerializer: typeof(PrototypeIdSerializer<EntityPrototype>))]
    public string PacketPrototype = "SeedBase";


MilonPL · 2025-01-31T15:28:01Z

Content.Server/Botany/SeedPrototype.cs

    /// </summary>
-    [DataField("productPrototypes", customTypeSerializer: typeof(PrototypeIdListSerializer<EntityPrototype>))]
+    [DataField(customTypeSerializer: typeof(PrototypeIdListSerializer<EntityPrototype>))]
    public List<string> ProductPrototypes = new();


github-actions · 2025-02-08T23:39:58Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

MilonPL · 2025-04-18T13:10:42Z

Thank you for your contribution! Due to this PR being having no activity and unresolved review comments, we've decided to close it. If you ever come back to it, feel free to open a new PR.

github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 15, 2024

PraxisMapper added 10 commits September 19, 2024 15:01

Update visualizers

778971d

The core plant entity definition

6ce1582

Move Plant stuff out of PlantHolder

b04b330

Small tweaks to related systems

45daaec

Remove Unique, IdealLight, LightTolerance. Other cleanup.

3937ecf

Update reactions for plants

1c74272

bugfix for entity definition

a7acb5b

Update mutation effects

feeeaf3

Update chemical effects

e68cb5a

Clamp plantholder values.

40bf2e4

drakewill-CRL force-pushed the plantasentity branch from 41c86d1 to 40bf2e4 Compare September 19, 2024 23:33

PraxisMapper added 3 commits September 19, 2024 19:43

removing light growth stats from seeds

f24e84e

removing light stats from salvage map

84d68b0

yml formatting fixes

f5f0fc7

drakewill-CRL marked this pull request as ready for review September 19, 2024 23:51

drakewill-CRL requested a review from Partmedia as a code owner September 19, 2024 23:51

github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. label Sep 19, 2024

Partmedia added the A: Service Area: Service department, including cooking, botany, etc label Sep 20, 2024

Apply plant mutation effects at planting if the seed had any

44daf5c

drakewill-CRL mentioned this pull request Sep 20, 2024

Mutation effects that apply to plant should apply when planted #32258

Closed

2 tasks

convert connecting uids to nullable to fix map-save test

e80f0b9

PraxisMapper added 7 commits October 30, 2024 17:12

remove unused SplatPrototype variable

1c8ddcc

one more newline removed

e001ef5

Fix AutoHarvest

25ffc14

fix seed extractor

56a0302

keep effect mutations when mutating species

78fff9a

another args.Handled check, shorten DoHarvest

e6db326

Dead plant remove-on-harvest was not a bug.

e4f0280

github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Nov 4, 2024

Merge branch 'master' into plantasentity

dca7b7b

github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Nov 4, 2024

PraxisMapper added 3 commits November 3, 2024 22:16

include Ligneous popup

ebeffc1

Merge branch 'plantasentity' of https://github.com/drakewill-CRL/spac…

c8fcc58

…e-station-14 into plantasentity

remove duplicate persists:false entry

7f02639

SlamBamActionman added the S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. label Nov 14, 2024

Emisse removed the Changes: Map Changes: Might require knowledge of mapping. label Jan 19, 2025

MilonPL reviewed Jan 31, 2025

View reviewed changes

github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Feb 8, 2025

MilonPL closed this Apr 18, 2025

Pok27 mentioned this pull request Oct 16, 2025

[Re-open] Botany Rework Part 2: GrowthComponents #39311

Open

2 tasks

Uh oh!

Botany Rework Part 3: Plants as Entities #31941

Botany Rework Part 3: Plants as Entities #31941

Uh oh!

Conversation

drakewill-CRL commented Sep 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

About the PR

Why / Balance

Technical details

Media

Requirements

Breaking changes

Uh oh!

mirrorcult commented Sep 8, 2024

Uh oh!

nymhole commented Sep 12, 2024

Uh oh!

drakewill-CRL commented Sep 12, 2024

Uh oh!

github-actions bot commented Sep 15, 2024

Uh oh!

DTSpawn commented Nov 2, 2024

Uh oh!

github-actions bot commented Nov 4, 2024

Uh oh!

Qwazori commented Jan 7, 2025

Uh oh!

drakewill-CRL commented Jan 9, 2025

Uh oh!

slarticodefast commented Jan 9, 2025

Uh oh!

MilonPL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 8, 2025

Uh oh!

MilonPL commented Apr 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

drakewill-CRL commented Sep 8, 2024 •

edited

Loading