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

Fix inherited parallax values causing layers to display incorrectly #3125

Merged
merged 2 commits into from
Aug 31, 2021

Conversation

LilithSilver
Copy link
Contributor

Fixes #3124.

Before, parallax was causing group layers to be offset as well as non-group layers. Now, only non-group layers have the parallax offset, so that they can properly use their effective inherited parallax value.

Visually tested on the scene I included in the issue. Also, tested with a 2-layer inheritance: 0.8 * 0.5 * 0.5 = 0.2.

LilithSilver and others added 2 commits August 30, 2021 16:30
@bjorn
Copy link
Member

bjorn commented Aug 31, 2021

Good catch and nice fix!

Since the layer item position is actually set in three places (on creation, when updating all positions and when responding to changes in layer properties), I've introduced a helper function with the group layer check.

I considered whether it would be possible to use the local parallax offset instead of the effective one, but it seems difficult to obtain the same behavior that way. Could it be that the current behavior is not intuitive at all? Maybe parallax factors should be summed up instead of multiplied (after subtracting 1). I wonder if anybody would be relying on the multiplication behavior already...

@bjorn bjorn merged commit fd36059 into mapeditor:master Aug 31, 2021
@LilithSilver
Copy link
Contributor Author

Sounds good, thank you!

Personally, I think multiplication is pretty intuitive. It's exactly how I would expect it to work -- something going slower in the background should go even slower if I made their parent go slower, and vice versa. I think where it gets confusing is with negative values -- a negative times a negative is a positive! -- but negative values aren't super useful, and the user can always make sure their group structure accounts for that. Or perhaps a multiplication strategy like -2 * -0.5 = - 1 is in order, for those; I'm not sure.

It's also pretty easy to implement with multiplication; I just added it to my renderer for my game and it really wasn't bad!

@bjorn
Copy link
Member

bjorn commented Sep 1, 2021

@LilithSilver Thanks for sharing your thoughts! One thing that bothered me about the multiplication, is that once you put a group on factor 0, you can't make any of its child layers move, but I guess that's unlikely to bother anyone in practice. I think alternatively, it could behave like any value set on a child layer would override the value of the parent rather than multiply by it (but of course then we need to introduce an "unset" state).

@LilithSilver LilithSilver deleted the fix-parallax-inherit branch September 1, 2021 08:59
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.

Incorrect preview position with inhereted parallax values
2 participants