ldtteam / Structurize

Minecraft Structures
GNU General Public License v3.0
45 stars 45 forks source link

Fix blueprint rotation change upon selecting new blueprint #592

Closed Thodor12 closed 1 year ago

Thodor12 commented 1 year ago

Closes #389 Closes ldtteam/minecolonies#8719

Changes proposed in this pull request:

Review please

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

uecasm commented 1 year ago

Why does this want to do the rotation and mirror in the opposite order from how everything else everywhere does it? That seems suspicious, since rotate then mirror produces a different result from mirror then rotate.

Thodor12 commented 1 year ago

Why does this want to do the rotation and mirror in the opposite order from how everything else everywhere does it? That seems suspicious, since rotate then mirror produces a different result from mirror then rotate.

This only separates the rotation and mirroring, just like you can do if you were to rotate and then mirror by clicking the buttons on the build tool. The actual rotation and mirror logic is still set in that method, and that hasn't changed (the rotateWithMirror method)

The issue is that the method fails if the rotation and mirror are changed simultaneously, I spent some time trying to figure out why but eventually fell back to this as Ray also didn't know this old implementation so well anymore either.

Nightenom commented 1 year ago

It looks wierd to call the method twice, please don't merge, will look myself and merge next week

uecasm commented 1 year ago

This only separates the rotation and mirroring, just like you can do if you were to rotate and then mirror by clicking the buttons on the build tool. The actual rotation and mirror logic is still set in that method, and that hasn't changed (the rotateWithMirror method)

Yeah, but inside that method it does the mirror first and the rotate second. It also seems weird to only change this one call of it when it's used in a huge number of places.

Thodor12 commented 1 year ago

The specific use case for this is, when you rotate and mirror a schematic, then proceed to choose another schematic, this method will want to rotate and mirror the schematic at the same time. This functionality breaks and will actually render the schematic without any rotation/mirror applied to it, whilst the blueprint rendered still thinks it has a rotation/mirror. This causes the preview and actual schematic to run out of sync, if you then proceed to paste the schematic it will actually revert to the original intended destination.

This is what players were seeing, thinking that the schematic rotation itself was broken.

Raycoms commented 1 year ago

It looks wierd to call the method twice, please don't merge, will look myself and merge next week

@Nightenom done already?