-
-
Notifications
You must be signed in to change notification settings - Fork 311
fix: improve biome setting to avoid writing directly to chunk #2757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Removes possibility of writing to the LevelChunkSection biomes PalettedContainer whilst it is being read for sending packets - I believe this occured mostly on clipboard operations where blocks are written before biomes, so chunks are being sent whilst writing biomes - This would explain why the error reported in the below issue (and others) is/was so rare - Of course I could be completely wrong about all of this, but given the line in LevelChunkSection#write that the error seems to consistently occur on is when writing biomes to the packet, and that the only place I can find in FAWE where we write to a "live" PalettedContainer is for biomes, I am reasonably confident that this is the cause - Should address #2729
|
From my understanding, the linked issue is caused by the fact that we manipulate the
So if we change anything about the Am I missing something? |
|
Ah yeah that does make more sense. The changes here are ones that should probably go in anyway so I'll edit the PR desc |
| Field tmpFieldBiomes; | ||
| try { | ||
| // It seems to actually be biomes, but is apparently obfuscated to "j" | ||
| tmpFieldBiomes = LevelChunkSection.class.getDeclaredField("biomes"); | ||
| } catch (NoSuchFieldException ignored) { | ||
| tmpFieldBiomes = LevelChunkSection.class.getDeclaredField("j"); | ||
| } | ||
| fieldBiomes = tmpFieldBiomes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops one more question, why isn't this using Refraction like the other fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// It seems to actually be biomes, but is apparently obfuscated to "j"
MiniMappingViewer said j but when actually using it, it was biomes for whatever reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you tested that on paper, you probably encountered the remapped version - but paper should also successfully remap j to biomes then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but got fieldNotFound exception so idk what was up. Didn't bother to look into it further than this fixed it tbh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works for me on 1.20.6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was on 1.20.4, I also didn't bother checking every version, just assumed it could be an issue there as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also works on 1.20.4 for me, Refraction properly detects that it's running in a mojmapped environment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll stick it back to normal and assume it was something off just once
Hacky workaround by overwriting the text3 StyleSerializer class with our own modified copy. Note that the hover events "show_entity" and "show_item" aren't supported as they no longer take a rendered component, but just the NBT structure. Behavior remains unchanged if 1.21.4- is detected. Fixes #2756.
* Merge pull request EngineHub/WorldEdit#2740 from ssquadteam/version/7.3.x Add ability to copy state string from info tool Co-authored-by: ssquadteam <sahib.2009.sa@gmail.com> * Update enum-like classes with 1.21.5-rc1 data from MCUtils * Do not paste unsaveable entities in their default state (#2721) Remove a redundant passenger check, as entity.save() returns false in that case. This also causes leash knots to not be copied. I don't think this is a problem because: - They would not be saved to disk, it's misleading for users that they appear. - Pasted leashed mobs still think they're leashed to the original position and get unleashed* - no change in behaviour. \* Unless they're pasted close enough to the original position, in which case this has better behaviour because they create their own leash_knot entity. * Update click and hover text component serialization for 1.21.5. (#2757) Hacky workaround by overwriting the text3 StyleSerializer class with our own modified copy. Note that the hover events "show_entity" and "show_item" aren't supported as they no longer take a rendered component, but just the NBT structure. Behavior remains unchanged if 1.21.4- is detected. Fixes #2756. * Add text3 bukkit adapter override for Spigot gson change. (#2759) * Make SpigotAdapter compatible with pre 1.21.5 api --------- Co-authored-by: Octavia Togami <octavia.togami@gmail.com> Co-authored-by: ssquadteam <sahib.2009.sa@gmail.com> Co-authored-by: Maddy Miller <git@madelinemiller.dev> Co-authored-by: brickmonster <92665597+brickmonster@users.noreply.github.com> Co-authored-by: wizjany <wizjany@gmail.com> Co-authored-by: SirYwell <hannesgreule@outlook.de>
Uh oh!
There was an error while loading. Please reload this page.