Crash Report - Cubic Chunks

[Xorbah]

I know CC is rather incompatible with many other mods but figured this was worth a post to see if a resolution could be found. Here's the crash log.

crash-2019-03-27_03.43.30-server.txt

[Barteks2x]

The cubic chunks side of the report: https://github.com/OpenCubicChunks/CubicChunks/issues/433

After looking into it a bit more, this is not going to work as long as this mod is extending that vanilla class and overriding almost all of it's methods.

This looks like just an attempt to avoid coremods, where the alternative is even worse.

There are a few different "edited" classes that are edited by completely replacing their instances, that will also make it incompatible. There is no way for me to make it work without this mod being a coremod, or submitting the right APIs as forge PR.

[Xorbah]

This is unfortunate news.

On Wed, Mar 27, 2019, 7:37 AM Bartosz Skrzypczak notifications@github.com wrote:

The cubic chunks side of the report: OpenCubicChunks/CubicChunks#433 https://github.com/OpenCubicChunks/CubicChunks/issues/433

After looking into it a bit more, this is not going to work as long as this mod is extending that vanilla class and overriding almost all of it's methods.

This really looks like yet another "but coremods are bad" case, where in this specific case coremod may actually be a better solution. The fact that it's possible to make it something not a coremod doesn't mean that it's always a good idea.

There are a few different "edited" classes that are edited by completely replacing their instances, that will also make it incompatible. There is no way for me to make it work without this mod being a coremod, or submitting the right APIs as forge PR.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Laike-Endaril/Dynamic-Stealth/issues/40#issuecomment-477132830, or mute the thread https://github.com/notifications/unsubscribe-auth/AbtGNCeJ_MtXzG7LCKC-r3_K2KnqFNxUks5va2YCgaJpZM4cNPkE .

[Laike-Endaril]

Unfortunately we each need our own custom entity trackers for each of our mods. Luckily in the case of DS, this only applies if you want the player senses feature.

If you disable player senses it will prevent this particular crash. @Xorbah Try setting (server settings -> senses -> use player senses) to false.

I may look into this further at some point, but it's not high on my priority list right now. I'll keep this issue in the tracker for now though.

[Laike-Endaril]

I did some quick testing and was able to sneak up behind zombies at a height of 300 blocks without throwing anything in the log (with player senses disabled), so as long as you're ok with DS's senses system only applying to non-players you should be good to go.

Edit: By which I mean players will be able to see "normally", like in vanilla. Mobs will still use the senses system fully and will not see other mobs or players unless DS says they can.

[Barteks2x]

Unfortunately overwriting mob AIs is going to cause slow downs as you go higher and some AIs are likely to break below y=0 because you would be reverting my patches.

I was considering making this entity tracker part a mixin where it would still run but would silently break my entity tracker patches.

You are effectively doing what a coremod would do but in a way that is much less compatible with other coremods that change the same class.

[Laike-Endaril]

Interesting...but yeah, I haven't even taken a peek at coremodding yet, and have a lot of other higher priority things to work on atm unless this compat issue turns out to be relevant with a larger number of mods. This would take some significant time just for me to research things before even starting to make changes, and so far this kind of issue has only been reported between DS and CC, so for now it's on the backburner.

Right now I need to do optimizations on DS, and finish a full menu system for Set Bonus, and then I have a few years worth of other TODOs :P

Basically I'll come back to this only if...

1. It turns out to be a bigger issue than I think it is (ie. compat with a larger number of mods)
2. I end up learning coremodding for some other reason, and decide this fix will no longer require as much time to implement

[Barteks2x]

For most mods writing a coremod is considered a bad idea, but here i think it would actually be a better (even if still far from ideal) solution. At this scale using Mixin may actually look like a good idea, because this is a significant amount of changes.

I don't think it will be a big issue for most mods, but it will likely cause issues with sponge.

[Barteks2x]

For sponge compatibility, this is what sponge is doing that this mod is going to completely overwrite and likely cause silent (or less silent) failures: https://github.com/SpongePowered/SpongeCommon/blob/74573a2d0af6facdabfa58973edf1ad2eb181024/src/main/java/org/spongepowered/common/mixin/core/entity/MixinEntityTracker.java Most of this package: https://github.com/SpongePowered/SpongeCommon/tree/74573a2d0af6facdabfa58973edf1ad2eb181024/src/main/java/org/spongepowered/common/mixin/core/entity/ai

Those are just what I was able to quickly find. And while cubic chunks is not that popular (yet?), sponge compatibility is likely to be something more people want.

[Xorbah]

I can't wait until CC gets world painter support, personally. But off topic. Hope this particular issue gets resolved someday.

On Thu, Mar 28, 2019, 3:09 PM Bartosz Skrzypczak notifications@github.com wrote:

For sponge compatibility, this is what sponge is doing that this mod is going to completely overwrite and likely cause silent (or less silent) failures:

https://github.com/SpongePowered/SpongeCommon/blob/74573a2d0af6facdabfa58973edf1ad2eb181024/src/main/java/org/spongepowered/common/mixin/core/entity/MixinEntityTracker.java Most/the whole this package: https://github.com/SpongePowered/SpongeCommon/tree/74573a2d0af6facdabfa58973edf1ad2eb181024/src/main/java/org/spongepowered/common/mixin/core/entity/ai

Those are just what I was able to quickly find. And while cubic chunks is not that popular (yet?), sponge compatibility is likely to be something people want.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Laike-Endaril/Dynamic-Stealth/issues/40#issuecomment-477752398, or mute the thread https://github.com/notifications/unsubscribe-auth/AbtGNKG265eBaczpUxZ4eqMYbiE2Q3Nzks5vbSGKgaJpZM4cNPkE .

[Johni0702]

This likely also affects Better Portals: https://github.com/Johni0702/BetterPortals/issues/191 Disabling server settings -> senses -> use player senses fixes the issue.

BP doesn't mixin into EntityTracker, it does however depend on accessing its internal entries set to move players from one dimension to another and your overwrite is duplicating it, thereby breaking BP which still uses the original one. So changing your edits to happen via asm/mixin will likely also fix this issue (though I'm not 100% sure of that since I haven't looked your edits). See https://github.com/Johni0702/BetterPortals/blob/d8072440f0bc336a93c2798cf50c7b3e76f5e285/src/view/kotlin/de/johni0702/minecraft/view/impl/server/ServerViewImpl.kt#L151-L167

[Laike-Endaril]

@Johni0702 I'm pretty sure that one can be fixed without ASM. In fact, I'm pretty sure changing to ASM would not have fixed it either, but I'd have to test a certain compat system I haven't created yet to be sure (related to the entity tracker and how it handles certain entities). I've created a new issue for it here for now.

Btw, in case the original posters are getting notifications and checking in, I have tinkered with ASM a bit, but still no ETA on this issue. I've only done a tiny bit of ASM, and am currently prioritizing other things, "other things" being whatever the server dev team I joined needs done.

[Johni0702]

In fact, I'm pretty sure changing to ASM would not have fixed it either

It looks to me like the only reason you duplicated the EntityTracker.entries and EntityTrackerEntry.trackingPlayers sets is that the original ones are private. Using ASM/Mixin, you'd be changing the EntityTracker/EntityTrackerEntry classes directly, so you'd be able to access the original sets as well, i.e. you wouldn't have to duplicate them.

I'm pretty sure that one can be fixed without ASM.

One simple way would be retrieving the original set from the base class via reflection instead of creating a new one but that's really just one hack on top of another hack.

[Barteks2x]

This mod will probably no longer crash with latest version of CC as I moved my changes to entity tracker to mixin, but the CC changes to EntityTracker won't work either. This is likely to cause some issues.

[Laike-Endaril]

@Johni0702 I was thinking of one other possibility, but what you just mentioned could be the issue as well; I haven't looked at it in a while and forgot those were private...but I believe I should've been replacing references to the original tracker so they point to my own instead, unless your code is storing a reference to a tracker internally, before my replacement occurs, or unless I missed an existing reference somewhere... I'm not good with kotlin btw. Also, I created the new issue for yours so I don't have to do a split conversation in this thread or ping Barteks with every response (though in this case I'm responding to both of you anyway). If you don't make any changes to the entity tracker directly, then ASM should not be required for compat. I might take a quick look at my replacements and timings today

@Barteks2x I concur; I'd like to eventually switch to ASM...or possibly mixins, though it has its own set of issues, like loading the first version of mixins encountered and using that for all mods, even if a later-loaded mod needs a higher version, which can cause crashes or other failures :/ In any case, planned for "someday", now that I've at least tinkered with ASM a bit