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

Add Parallax Origin property to Map Properties #3209

Merged
merged 6 commits into from
Dec 17, 2021
Merged

Conversation

krukai
Copy link
Contributor

@krukai krukai commented Dec 15, 2021

Independent from the plans for viewports, giving the user means to change the parallax offset (as in a "starting position") would help greatly to accommodate differences between Tiled's approach and that of a user's game project.

Please note that this is still WIP; the PR was created for enabling discussion (and bug fixing).

src/libtiled/mapreader.cpp Outdated Show resolved Hide resolved
src/tiled/propertybrowser.cpp Outdated Show resolved Hide resolved
@eishiya
Copy link
Contributor

eishiya commented Dec 15, 2021

Rather than "parallax offset", which to me suggests the calculated draw offset based on parallax, I think the property should be called something like "parallax origin" or something else that suggests that it's a reference point.

@krukai krukai changed the title Add Parallax Offset property to Map Properties Add Parallax Origin property to Map Properties Dec 16, 2021
* Avoid the need to negate the origin value, which probably made sense
  when it was still called an "offset", but as an "origin" I think it
  should refer to a point on the map.

* Update the layer positions immediately when the parallax origin is
  changed.
@bjorn
Copy link
Member

bjorn commented Dec 16, 2021

I've changed the parallax origin to get subtracted, rather than added, to the view center. It seemed more intuitive to me, since I interpret the parallax origin to be a point on the map. Does that make sense?

I've also fixed the immediate updating of the layer positions when you change the property, by connecting the MapScene to the Document::changed signal and checking for this property.

The change looks fine to me now, but before we merge this we need to again update the documentation:

  • Add property to index.d.ts.
  • Add property to JSON and TMX format documentation, including their changelog.
  • Update the https://doc.mapeditor.org/en/latest/manual/layers.html#parallax-reference-point section in the documentation. It now needs to mention the parallax origin, how it is relative to the top-left of the map's bounding box, and after that can mostly replace "map’s origin" with "parallax origin".

@krukai
Copy link
Contributor Author

krukai commented Dec 16, 2021

I think I covered it all. I did not test all exports yet as I am waiting for some additional feedback on whether the origin behaves as expected - I do not trust my own sanity checks alone.

@krukai
Copy link
Contributor Author

krukai commented Dec 16, 2021

That should be all, I think. Please have a final look.
Cheers!

* Tried to make the documentation a little more clear.

* Added version to JSON format docs.

* Simplified code in VariantToMapConverter::toMap.
@bjorn bjorn merged commit b1450db into mapeditor:master Dec 17, 2021
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.

3 participants