-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 tileset tile x/y draw offset. #3488
base: master
Are you sure you want to change the base?
Conversation
The main issue I can see with this is the lack of tooltip, translations and some way to know if a block has been offset in some way inside the tileset display. Additionally, not all map formats will support this new functionality, but if you know of any that do, I can update the exporters as well. |
Would it be better to use the "tilesetTileOffsetChanged" rather than the "tileImageSourceChanged", neither are perfect of course, but I am not sure if should be adding new signals. |
@bjorn Is there anything else needed for this? |
@Chase-san I appreciate that you're picking up this change again! I will have a look at this tomorrow, but at a quick glance I could not see much difference from the changes at #1323 so it'd probably be fair to look into merging that PR instead. Is there anything in this PR that the older one didn't cover? |
@bjorn This one is up to date with the codebase and provides a more robust map save/load functionality. It differs slightly in how it applies the offset. If you would like more to be changed for this functionality to be better integrated, please let me know. |
I think this should be more intuitive, especially when we later make this property visually editable.
The enums are not saved or encoded into a file at any point right? Seen that be an issue before, which is why I put the new values at the end. |
If a tile only had its origin customized, it would still fail to get written out.
Right, these enums are only used within the application. :-) What remains to be fixed before this can be merged, is fixing the rendering artifacts that can appear due to the tile not occupying the area expected by for example the brush preview. We may also need a more specific change signal than |
Do you have an example of the artifacts? |
@Chase-san Sorry, I ran out of time and today again I ran out of time, but regarding the artifacts, the steps I can reproduce it with are:
There is already code to avoid such artifacts, which takes into account the tile offset set on a tileset. This will need to be adjusted, so that also individual origins on tiles are taken into account. |
Sorry I've been pretty busy with other things, I'll take a look at this this weekend. |
@Chase-san No worries, I'm currently looking into fixing this up. |
The tile origin changes where the tiles are rendered, so it needs to be taken into account when calculating the draw margins. These are used when determining which area to repaint, and which tiles to render in a certain area. The draw margins are cached per map and now also per tileset. They are invalidated when the "tile offset" or a tileset is changed as well as when the "origin" of a tile is changed.
Still to do:
|
I've decided to de-schedule this change from Tiled 1.10 since it will need more time. As an individual "drawing offset" setting it wasn't very problematic, since we already supported that on the tileset level. However, I've changed this property to "origin", which I thought was just a negation and simple rename, but it actually affects the interaction with alignment for both tile objects and tiles placed on a tile layer, which are not behaving intuitive in its current state due to the default bottom-alignment in both cases. This issue remains high priority, but we need to take the time to do it right, preferably simplifying things rather than making things more complicated. |
How usable is this commit at this juncture? Per-tile offset / origin is important to me too so if the progress of this feature is on hold, I'll be happy just using something minimal instead. |
Unfortunately I had to take a rather long (over 2 months) break from Tiled development recently, and it's a bit hard to tell for me which state this was in. There were some issues raised (see my previous comment), but it's been a while and I need to get back into it, play around with the current state and then decide what way to go forward. |
From what I could grok, there seems to be an alignment issue with previews? Maybe it would help if we could have a list of things to be fixed (and where to find them in the code) for anyone unfamiliar with the codebase who wants to jump in and help with the PR? |
While that may sound reasonable on the surface. If that information was readily know they would simply fix it. |
Very well. That's kind of unfortunate... On a side note, were you able to use this commit to any success for your game? |
@Favouris Since the previous builds had expired, I've merged |
This is the missing feature, first asked 9 years ago 😢. Hope this will we merged soon. |
Thanks for this.
Will keep the notes here as I test |
This is a modern(ish) version of #1323. I just want to do the minimum needed to get this merged in, whatever that minimum might be. I want to avoid making the PR overly complex, so it is very bare bones at the moment.
This functionality, or something like it is important for a game I am working on.