Skip to content
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

Odd summetry fix take2 #405

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

clivepaterson
Copy link
Contributor

So been working on this for a while, managed to get the reverse lookup working.
Results are really good, and any map that is generated with Perfect Symmetry is unaffected by any of these code changes.

Note: I had to change the setSizeInternal() function, because the use of applyWithSymmetry() was causing issues.
So I simplified it, a lot, and it is the same efficiecy, maybe more efficient.

The rest of it relies on the copyPrimitiveFromReverseLookup() function, which logically divides the map into slices, and for the 2nd, 3rd, 4th, etc slice, it copies it's value from the 1st slice.

This is a look at a 7 player map generated with these changes:
image

Copy link
Member

@Sheikah45 Sheikah45 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick comment then will take a deeper look later

Comment on lines 232 to 238
Map<Integer, Integer> coordinateMap = getSymmetricScalingCoordinateMap(oldSize, newSize);
applyWithSymmetry(SymmetryType.SPAWN, (x, y) -> {
boolean value = getBit(coordinateMap.get(x), coordinateMap.get(y), oldSize, oldMask);
applyAtSymmetryPoints(x, y, SymmetryType.SPAWN, (sx, sy) -> setPrimitive(sx, sy, value));

float scale = (float)oldSize / (float)newSize;

apply((x, y) -> {
int sx = (int)(x * scale);
int sy = (int)(y * scale);
setPrimitive(x, y, getBit(sx, sy, oldSize, oldMask));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One note here is that we still need the symmetricScalingCoordinateMap, otherwise the map will end up with a one pixel shift in some parts.

This is because the conversion from float to int is not symmetric around the center of the map. Sof for example with a ratio of .5 11->5 while (100-11)->(50-5.5)->(44.5)->44 when really we would want it to go to 45 to have the same distance from the edge assuming a 100 pixel size map to a 50 pixel size map.

Copy link
Contributor Author

@clivepaterson clivepaterson Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yep, I see what you mean.
I checked this scenario, scaling from 101 down to 50, and it was shifting 1 pixel to the left. Classic.
I guess to scale these ones properly, I'd have to scale from the middle of each pixel, ie add 0.5 then scale then subtrct 0.5

I've reverted this to use the getSymmetricScalingCoordinateMap() again.
But used the apply() function to ensure each pixel is scaled onto the new mask.

The applyAtSymmetryPoints() has problems because it skips some pixels during scaling, and it sets some pixels twice:
So applyAtSymmetryPoints() looks like this:
image

…, but use apply() instead of applyWithSymmetry().
Copy link
Member

@Sheikah45 Sheikah45 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than putting an if statement in a lot of the random functions would it instead be possible to change applyWithSymmetry itself so that if it is perfect symmetry it works as it does now, and if it is not then it fills in the root slice and then uses copyFromReverseLookup.

This is better from an API standpoint so that individual functions don't have to know how to keep symmetry.

Additionally it may be good to just note that the reverse lookup only works for rotational symmetries.

@Sheikah45
Copy link
Member

It should be able to effectively replace the if check that already exists in applyWithSymmetry

@clivepaterson clivepaterson marked this pull request as draft August 4, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants