search

anegostudios / VintageStory-Issues

Vintage Story's public issue tracker for reporting bugs, crashes and the like
44 stars 16 forks source link

BlockEntity reinstantiated on block exchange #3935

Open danger-dan opened 3 months ago

danger-dan commented 3 months ago

Game Version

1.19.8

Platform

Linux

Modded

None

SP/MP

Multiplayer

Description

Sometimes when block is exchanged the block entity is reinstantiated. Block exchange is meant to swap out the block without destroying and recreating the blockentity. I noticed this when working with snow updates. This happens rarely and never when exchanging a non snow block -> snow block only when exchanging the snow block with the non snow block.

How to reproduce

Here is a minimal reproducable mod that can show the problem.

https://github.com/danger-dan/vs_bug

  1. Run server and client - connect to a world.
  2. Set world to creative mode (You may need to change permissions)
  3. place blocks found by searching 'bug'
  4. Make it rain
  5. cycle between december and july using /time setmonth dec/jul
  6. Notice the logs - sometimes when going from snow -> no snow, you will see 'Constructor' in the logs. As far as I'm aware this shouldn't be happening.

Screenshots

No response

Logs

Log

Craluminum2413 commented 2 weeks ago

You certainly don't exchange block in your code

danger-dan commented 2 weeks ago

Did you even test the this? Snow update calls block exchange. I could provide an example explicitly using block exchange if that would be preferred - but that shouldn't matter. I used the snow update to test the example as that was the problem I ran into. I suspect anything calling block exchange would cause this bug to appear.

Craluminum2413 commented 2 weeks ago

@danger-dan Yes, please

danger-dan commented 1 week ago

I added some code that calls the exchange block on a tick timer. It does not reproduce the problem. I did some more digging, the issue is CLIENT side ONLY. The server doesn't lose the entity. So i started thinking maybe its a threading issue. Both the tick listener and snow update are called in main thread - so its not that. I can't dig any further without seeing the source code. My gut feeling is the client side code has got some weird edge case bug specifically around snow variants when being exchanged. But i could be wrong.

Please don't close this issue because it is not reproducible calling exchange directly. It is a problem and I can reproduce it 100% of the time. It makes any snow variants that use entities UNUSABLE.

Craluminum2413 commented 1 week ago

@danger-dan You always need to call MarkDirty i guess

danger-dan commented 1 week ago

@Craluminum2413 That may fix the lost properties yeah. However calling the 'ForceExchange' function doesn't cause the issue (which doesn't call mark dirty) so I think its something fundamentally broken, also the entity is being reinstantiated which just shouldn't be happening regardless. Overriding PerformSnowLevelUpdate and adding a MarkDirty(true) call may be a work around for now. I will test that.

danger-dan commented 1 week ago

Disregard my above comment. Mark dirty doesn't do anything in this specific example as I explicitly don't override From/ToTreeAttributes to demonstrate the problem. However in a real use case one would override and save the attributes. The server will of course then send the attributes - when the new entity is constructed. The actual issue being: snow update calls block exchange and shouldn't be destroying and creating a new entity. However calling BlockExchange directly doesn't cause the issue. VERY ODD.