The Wild Mod [Fabric]

The Wild Mod [Fabric]

658k Downloads

[Suggestion] Abstract out growing mechanics and block checks from the grower classes

oliviathevampire opened this issue ยท 4 comments

commented
commented

I have a quick question- do you mean using the same checks for both Sculk&Activator? If so, that'd result in code clutter, given that the Activator's SculkCheck is different from the normal SculkGrower's check, I wouldn't see how moving it into another class would improve anything.
If you mean to do use the same checks with CatalystThreader and the two Grower classes, I'm actually planning to remove CatalystThreader as it simply doesn't work properly anymore. CatalystThreader also needs different growing code because using multiple threads to change any blocks in the world causes the game to crash, so I need an altered version of the growing code for it to work (although this won't matter once I remove it.)
There could be something I'm missing or don't know about, so feel free to correct me.
EDIT: I might keep the threaded Activator Growth actually, as that hasn't caused anything to break and frees up a bit of performance.

commented

I have a quick question- do you mean using the same checks for both Sculk&Activator? If so, that'd result in code clutter, given that the Activator's SculkCheck is different from the normal SculkGrower's check, I wouldn't see how moving it into another class would improve anything. If you mean to do use the same checks with CatalystThreader and the two Grower classes, I'm actually planning to remove CatalystThreader as it simply doesn't work properly anymore. CatalystThreader also needs different growing code because using multiple threads to change any blocks in the world causes the game to crash, so I need an altered version of the growing code for it to work (although this won't matter once I remove it.) There could be something I'm missing or don't know about, so feel free to correct me. EDIT: I might keep the threaded Activator Growth actually, as that hasn't caused anything to break and frees up a bit of performance.

i believe what was meant here was that we split the activatorgrower into its own thing that gets called upon by sculk sensors and shriekers, and would make it easier for us to let sculk jaws grow in twm+ since it would just use the abstract code

commented

i believe what was meant here was that we split the activatorgrower into its own thing that gets called upon by sculk sensors and shriekers, and would make it easier for us to let sculk jaws grow in twm+ since it would just use the abstract code

Just curious, how would that work, and how would that make growing Sculk Jaws easier? How would it get called by sensors and shriekers?

Either way this is exactly why I added the Death GameEvent, you can use a mixin for SimpleGameEventDispatcher, get the location of the GameEvent and check that the entity who died can drop XP, and run a JawGrower based on that.
The reason JawGrower would be separated is because it has different growing mechanics, it needs to replace Sculk instead of being above Sculk. You could also use tags to get rid of this problem entirely, which I'll talk about later. I don't necessarily see how doubling the code (and somehow getting it inside the Sensor's class) would improve or change anything.

On top of that, it'd likely cause two problems:

  1. Performance - You're not looping one method to see which activator to place, you'd be running two loops with twice the calculations.
  2. Crashes - Minecraft crashes when a BlockEntity tries to place itself in the same place as another. Splitting this into sensors and shriekers individually would likely allow for this to happen, as they're running at the exact same time in two different classes.

Personally, I think it may be a good idea to just create two new tags and use that for ActivatorGrower. You can choose a random block from tags (SculkTags.SCULK.getRandom()) and you can tell if a tag is empty (SculkTags.SCULK.values().isEmpty(),)
so I personally think this would make the most sense. It would require a few more if statements, but it would allow for easy compatibility and allow anyone who knows how to use datapacks to have some fun.

commented

I mostly meant seperate out the duplicated code which is the same between all of the growing stuff mostly.