KenRouKoro / C3H6N6O6

这是一个用于实体多线程运算的模组
MIT License
34 stars 1 forks source link

Incompatibility with ServerCore leading to a crash #6

Closed Aeldit closed 11 months ago

Aeldit commented 2 years ago

The server crashes when using this mod and servercore crash log : pastebin link (see line 156)

youmukonpaku1337 commented 2 years ago

oh damn i was about to report this, thanks

KenRouKoro commented 2 years ago

I see a code conflict between servercore and the C3H6N6O6 intercept entity tick, this is a principle level conflict and is not fixed by C3H6N6O6. This issue should be reported to the author of servercore.

youmukonpaku1337 commented 2 years ago

i just hope this'll be fixed on servercore's side because i use servercore for disabling spawn chunks completely and some other stuff

youmukonpaku1337 commented 2 years ago

@Raphoulfifou please link the issue here after you make it on servercore's repo, i want to keep track of progress

KenRouKoro commented 2 years ago

The reason for the crash is that ServerCore is forcing the Mixin check to succeed, and this problem requires turning off ServerCore's Mixin check.

youmukonpaku1337 commented 2 years ago

i guess you could make a PR on servercore for a toggle on the mixin check?

KenRouKoro commented 2 years ago

This is subject to the opinion of the ServerCore author.

youmukonpaku1337 commented 2 years ago

i'll create the issue myself

youmukonpaku1337 commented 2 years ago

done

Aeldit commented 2 years ago

ok so do I close this issue or do I just leave it like that ?

youmukonpaku1337 commented 2 years ago

wait a bit

youmukonpaku1337 commented 2 years ago

also, @KenRouKoro quote from that issue:

As a sidenote for the author of cyclonite - I might've missed it but is there any reason as to why it tries to run non-passenger entity ticks async, but it doesn't do the same for passenger entity ticks? Just curious.

KenRouKoro commented 2 years ago

In fact cyclonite loads all Entity Tick asynchronously.

youmukonpaku1337 commented 2 years ago

ah

Wesley1808 commented 2 years ago

I'm not sure where it does that, I only looked at ServerWorldMixin, for which it only seems to hook into tickEntity(), but not into tickPassenger(). Feel free to correct me if I'm wrong about this though.

KenRouKoro commented 2 years ago

I understand. The reason is this: there is no way to parallelize the method tickPassenger(), because the logic of the ride has to be sequential, unless I rewrite the whole ride system (which I obviously can't do).

KenRouKoro commented 2 years ago

The overhead of opening another thread to run it alone is theoretically greater than running it on the server thread, and given the actual gameplay, this method doesn't take up much computing weight (the entity that rides also has a separate tick, this method just handles the logic of riding alone)

KenRouKoro commented 2 years ago

However, it may be optimised later to run it after all entity ticks are in the queue.

Wesley1808 commented 2 years ago

That overhead indeed probably isn't worth it.

But if I'm not mistaken, Entity#tickRiding() does actually call Entity#tick(), not only riding logic. Meanwhile in the foreach loop on the entityList, before actually calling any tick logic it checks if the entity has a vehicle (is a passenger) - and if that vehicle is not removed and has the ticking entity as passenger (idk why it does a second check, probably some reason for that), it returns there.

Then again though, its only a small amount of entities so its probably not worth it.

KenRouKoro commented 2 years ago

Your description is probably an older version. The only specific tick that I can see in version 1.18.2 where I am is in tickPassenger(Entity vehicle, Entity passenger) is passenger.tickRiding(). The method tickPassenger() is called recursively.

KenRouKoro commented 2 years ago

There's also the fact that I've seen more than one pointless secondary check in Minecraft's code. For example, when checking if a message sent by the player is a command, it checks twice if the string starts with a "/", which is really confusing.

Wesley1808 commented 2 years ago

What I mean is that passenger.tickRiding() calls passenger.tick() itself. Not directly in the ServerWorld.tickPassenger() method itself. My apologies for the few that commented on this ticket and are now (maybe) getting 12 emails though 😆

KenRouKoro commented 2 years ago

Oh, crap, I missed it. But then again, it's better to leave the recursive call code to run on its own, after all, no one knows what will happen if it's intercepted, and MC has always been consistent in that regard. —————————————————————————————————————————————————————— I think Github needs to start a private chat feature lol

himekifee commented 2 years ago

Hook this. The old one was deprecated and caused minecart issues.