Problematic hive check logic for puffbugs

[Octelly]

Follow up to MarkusBordihn/BOs-Adaptive-Performance-Tweaks#72.

I am not technical enough to explain this. Please see this comment: MarkusBordihn/BOs-Adaptive-Performance-Tweaks#72 (comment)

[SmellyModder]

It looks like they are misunderstanding the issue. Puff Bugs are not an instance of Bee.
Puff Bugs were released before Mojang released bees.

This looks more like an issue on their side because they have a specific safety check for Bees, which probably suffer the same issue as the Puff Bugs do when special exceptions are not made for them.
This issue only happens when their mod is present because it optimizes the game at the cost of safety, with many notable exceptions built in for vanilla. However, Puff Bugs are not vanilla and are not on their exception list.

[MarkusBordihn]

Thank you for your feedback. Could you please clarify your statement a bit further?

According to the crash log, the Java runtime treats Puff Bugs as instances of bees; otherwise, the check entity instanceof Bee bee would return false and be skipped. This behavior isn't related to when the mob was first released.

Additionally, the crash is clearly happening in the linked code on your side, where the exception is not being handled.

It would also be helpful if you could elaborate on what you mean by 'optimizes the game at the cost of safety.' I'm more than happy to address any problematic areas and make improvements.

In general, it's not productive to handle issue reports without providing specific technical references to the relevant code and instead placing blame without deeper investigation.

[MarkusBordihn]

To help resolve this issue and move the discussion forward, I reviewed the code in detail and submitted a PR to address the problem: #264

The issue lies in the removeWhenFarAway method, which unnecessarily accesses chunk data. By replacing getHive() with getHivePose(), we can avoid this issue and any potential side effects. Additionally, getHivePose() is already checked regularly during tick events, triggering getHive() only when necessary.

This PR should resolve the reported issue, prevent other side effects, and slightly improve performance by checking only synchronized entity data rather than chunk data.

[SmellyModder]

The code demonstrates that Puff Bugs are not instances of Bees. As you've now noticed, the problem is the removeWhenFarAway method. I already saw this in the stack trace, but I thought my explanation of the problem was sufficient.

Yes, the crash is happening in my code, but your mod uses the removeWhenFarAway method in a way that Mojang did not design it to be used. This is why the crash only happens when Endergetic and your mod run together.

It would also be helpful if you could elaborate on what you mean by 'optimizes the game at the cost of safety.' I'm more than happy to address any problematic areas and make improvements. In general, it's not productive to handle issue reports without providing specific technical references to the relevant code and instead placing blame without deeper investigation.

I believed my explanation was technical enough. I'm explaining how performance mods generally work (i.e., the consequences of making assumptions to optimize the game). However, I will elaborate anyway since you asked. Your isRelevantEntity method has many checks it makes to ensure it properly checks for entities that are not relevant, but many of those checks are specific to vanilla. Puff Bugs are just one example of why this method is not sustainable; it doesn't consider specific modded entities that may cause problems. Sure, I could merge your PR, but you may have to make more PRs in the future because other mods have different assumptions. It's more sustainable for you to add a way to add modded entity types to the conditions in the method.

Also, even if I merge the PR, a new version will likely not be released soon due to our team's other priorities outside of Minecraft. Again, this is an example of why asking other mods to adjust to your new conditions is unsustainable.

I'm sorry if this comes off as mean, but I'm simply trying to explain that fixing this issue on our end is unlikely to benefit users immediately and represents an unsustainable fix for a pattern of issues like this one.

[Enterprise12nx01]

The code demonstrates that Puff Bugs are not instances of Bees. As you've now noticed, the problem is the removeWhenFarAway method. I already saw this in the stack trace, but I thought my explanation of the problem was sufficient.

Yes, the crash is happening in my code, but your mod uses the removeWhenFarAway method in a way that Mojang did not design it to be used. This is why the crash only happens when Endergetic and your mod run together.

It would also be helpful if you could elaborate on what you mean by 'optimizes the game at the cost of safety.' I'm more than happy to address any problematic areas and make improvements. In general, it's not productive to handle issue reports without providing specific technical references to the relevant code and instead placing blame without deeper investigation.

I believed my explanation was technical enough. I'm explaining how performance mods generally work (i.e., the consequences of making assumptions to optimize the game). However, I will elaborate anyway since you asked. Your isRelevantEntity method has many checks it makes to ensure it properly checks for entities that are not relevant, but many of those checks are specific to vanilla. Puff Bugs are just one example of why this method is not sustainable; it doesn't consider specific modded entities that may cause problems. Sure, I could merge your PR, but you may have to make more PRs in the future because other mods have different assumptions. It's more sustainable for you to add a way to add modded entity types to the conditions in the method.

Also, even if I merge the PR, a new version will likely not be released soon due to our team's other priorities outside of Minecraft. Again, this is an example of why asking other mods to adjust to your new conditions is unsustainable.

I'm sorry if this comes off as mean, but I'm simply trying to explain that fixing this issue on our end is unlikely to benefit users immediately and represents an unsustainable fix for a pattern of issues like this one.

their mod is used quite often in modpacks and their issue tracker has almost no issues. it has no issue interacting with hundreds of other mods that have new entities in all dimensions. this seems like a one off issue that many are experiencing with your mod. Considering he has even done the work for you is kinda insane imo, it takes a few clicks to make it part of a next update lol.

[Octelly]

@Enterprise12nx01 Hey, you might be technically correct, but it is their code to maintain and the decision is up to them. It is important to handle each issue properly so the code doesn't become unmaintainable.

[Enterprise12nx01]

@Enterprise12nx01 Hey, you might be technically correct, but it is their code to maintain and the decision is up to them. It is important to handle each issue properly so the code doesn't become unmaintainable.

well either way. it would be nice if it could be implemented, as he has already done a pull request..so it just has to be approved and posted, or even so we can compile it ourselves, as its sounding like your mods are currently abandoned