godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Add pivot origins other than top-left and centered to Sprite2D #9222

Open Nallebeorn opened 8 months ago

Nallebeorn commented 8 months ago

Describe the project you are working on

Describe the problem or limitation you are having in your project

The centered and non-centered (top-left) pivot origins of sprite is often not what I need (top-left is in fact pretty much never what I want). In particular, what I actually want is often center-bottom:

You can of course do this by manually adjusting offset. Setting it in the editor works, but requires you to look up the texture size yourself before typing it into the fields. It gets more annoying for sprites that animate or otherwise change their texture if the textures don't neccessarily have the same dimensions. In this case, you need a script to automatically update the drawing offset based on texture dimensions.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Replace the centered property on all the sprite nodes with a origin property that allows you to pick any corner or the center as the origin. Just like with centered, the offset property will then be relative to the origin preset.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I can see two approaches:

  1. Replace the centered bool with an enum for top-left, top-center, top-right, center-left, center, center-right, bottom-left, bottom-center, and bottom-right.
  2. Replace the centered bool with a Vector2 allowing any value from (0, 0) to (1, 1) -- ideally still with an option to pick from any of the 9 basic presets in the GUI for convenience.

I don't think picking a percentage-based base origin outside of the 9 basic choices would be very common, so 1) might make more sense even if it's technically less flexible.

I'm not sure how things like this are usually handled in Godot, but I believe it should be possible to implement in a backwards-compatible way by keeping centered as a deprecated wrapper property.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It's fairly easy to write a script that updates the sprite offset based on size in _process() or by reacting to texture change signals, but it's more cumbersome than a native implementation would be. The class hierarchy of sprite also means you might need duplicates of this script for AnimatedSprite3D, Sprite3D , Sprite2D , and AnimatedSprite2D respectively, if you want to be able to inherit and add additional behaviour on your sprite nodes.

Is there a reason why this should be core and not an add-on in the asset library?

This would work most smoothly as a change in the behaviour of the built-in sprite nodes rather than as an add-on built on top of them.

KoBeWi commented 8 months ago

You can update the offset from the editor, it doesn't need to be done using the inspector.

Nallebeorn commented 8 months ago

You can update the offset from the editor, it doesn't need to be done using the inspector.

Thank you, I didn't know that! Still, while that's very useful when you want to align the offset with a specific pixel I don't really think that's any easier than the inspector when you just want to move it from the center to the bottom (also not available for 3D sprites, I believe).

kromenak commented 4 months ago

100% agree with this, and actually came to post a similar proposal until I found this one!

Being able to offset a SpriteBase3D via pixels is fine, but it depends on the pixel size of the texture. If an art asset starts out at 1024x1024, but later changes to 512x512 for optimization reasons, I don't want the positioning to break.

The "centered" bool is also fine, but I want to be able to position the pivot at the bottom-center of the sprite. I think this is pretty common for things that walk or stand on the ground.

It seems like a very flexible solution is what was proposed - a Vector2 that uses values 0-1 (0% to 100%) to position the pivot from bottom-left to top-right. Values outside those ranges could even be used (if you want the pivot to be outside the texture for some reason).

In my opinion, it's kind of hard to justify ONLY having the "centered" option when there's a more flexible variable that can be exposed to handle pretty much any pivot positioning use-case. Perhaps you could argue that a boolean takes up less memory than 2 floats for a Vector2, but it seems like a drop in the bucket.

Nallebeorn commented 4 months ago

Values outside those ranges could even be used (if you want the pivot to be outside the texture for some reason).

I hadn't thought of that, but now that you mentioned it that might be surprisingly useful in certain cases! If you want to align a bunch of sprites in sequence next to each other, you could do that with pivot offsets that are multiples of 1 and not have to bother with using the size to calculate positions. That's somewhat besides the main point of the proposal, though :)

I'd also like to mention that while the title and label updates categorize this as for 2D, this would be useful for Sprite3Ds as well -- the project that made me submit this was even a 3D game using sprites for most characters/objects, even if 2D would be the most common case.

abe33 commented 4 months ago

I haven't commented here yet, but I've started working on a PR that implement a pivot mode field on sprites that replaces the centered property with an enum. The PR also provides support for pivots defined at the texture level (as available in spritesheets generated with tools such as texturepacker or aseprite).

I'd love to have your feedbacks on this.