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

Scaled Node3D and Node2D scene roots always show a warning #104282

Closed
MajorMcDoom opened this issue Mar 17, 2025 · 15 comments
Closed

Scaled Node3D and Node2D scene roots always show a warning #104282

MajorMcDoom opened this issue Mar 17, 2025 · 15 comments

Comments

@MajorMcDoom
Copy link
Contributor

MajorMcDoom commented Mar 17, 2025

Tested versions

Reproducible in 4.4.stable

System information

Godot v4.4.stable - Windows 11 (build 22631) - Multi-window, 1 monitor - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1660 SUPER (NVIDIA; 32.0.15.6636) - Intel(R) Core(TM) i7-10700F CPU @ 2.90GHz (16 threads)

Issue description

Image

I know this behaviour was probably added to 4.4 and intended, but I'm reporting it as a bug because IMO it is inappropriate. Rotated/scaled scene roots are a very legitimate use case and users should not need to be unconditionally warned about something they do on a regular basis without any means of turning off the warning. Seeing a bright yellow icon in the scene tree is very distracting, and if the user learns to simply ignore it, they risk ignoring legitimate warnings on the same node.

Many assets have a default root rotation or scale, which can optionally be overridden in containing scenes, or by inherited scenes. Some examples:

  • A book lying on its side
  • A cloud which has a default scale but can be made smaller or bigger

I think this warning might make slightly more sense if it were just in response to the position being non-zero, since that will always be overridden.

Steps to reproduce

  • Scale any Node3D or Node2D scene root.
  • Observe the warning that appears.

Minimal reproduction project (MRP)

N/A

@RobProductions
Copy link
Contributor

I feel like I always say this lol but it's kind of true here too, an editor setting to disable the warning would go a long way towards reducing the annoyance of it if we really had to keep it. But if we're not beholden to it, I'd also agree that it's pretty unnecessary. I see why it exists because there's some particular assumptions Godot makes when considering scale changes, but overturning the assumptions themselves could eliminate any confusion that caused the warning to be created in the first place.

Take a look at this CSG Box example scene which exists inside the main scene:
Image

And note how when I change the scale of the root itself, those changes are not reflected in the scene it exists within:
Image
Image

Yet the "reset icon" is there, so it knows that the scale has changed but Godot has chosen ignore it and instead enforce the previous scale even though it was previously matching the scene root scale. So to me it seems the most straightforward way of handling root scaling is that if you have already changed the Scene's scale from its default, it will keep the scale that was overridden and not use the new root scale. But if it was not overridden, it should scale with the scene changes.

What's weird is that other properties on the root don't behave this way; if you change them in their own scene they will reflect in the main scene. I'm not sure if this was an intentional choice or not, but it seems that root transform is handled differently and I would argue (at least for scale) that it should work the way the other properties do. Without that, artists can't scale assets properly and have that scale be applied to all instances of the asset without creating more children nodes.

@MajorMcDoom
Copy link
Contributor Author

MajorMcDoom commented Mar 17, 2025

Linking to #81892 for the original context.
Also tagging @aaronfranke , @KoBeWi .

I think it's a wrong assumption that all scenes are containers with scalable children. I have many scenes whose roots are transformed visuals. The consequence of this warning is that we're implicitly telling people scaled visuals require an extra layer in the hierarchy.

There are also many scenes authored at non-zero rotations, like books, control panels, tools, etc. Their "up" for logical purposes is not necessarily the same as their "up" for convenient placement. For example a book's Y axis goes along the page, but I author the book scene to be flat on its side, because that's how books are usually placed in scenes.

To put this into maybe simpler terms: I think there's a difference between a neutral value and a default value. I don't think it's correct to assume that a scene is always authored to be entirely neutral. In practice, people use it as a default.

My situation now is that I have a lot of perfectly fine things I've always done that the engine is now loudly telling me "This is probably not OK!".

@aaronfranke
Copy link
Member

This behavior is intentional (as you've noted). It's not always wrong, but it's not recommended, thus the warning. If it's annoying users too much, we could switch it to only checking the position. I won't die on this hill :)

Without that, artists can't scale assets properly and have that scale be applied to all instances of the asset

My recommendation is that artists scale up the child nodes or scale the meshes in Blender.

I have many scenes whose roots are transformed visuals.

But what happens when an instance of the scene overrides the transform? If a user sets a scene instance's scale to (2, 2, 2), they may expect it to be 2 times as big as the scene normally is. However, if the scene was already scaled at (10, 10, 10), setting such a scale would actually make it smaller by a factor of 5. Or, if a user sets the transform to identity, they probably expect it to be unrotated, unscaled, and at the same position relative to the original scene, but this is not the behavior that happens if the scene root has a transform.

There are also many scenes authored at non-zero rotations, like books, control panels, tools, etc.

In those cases I would recommend either rotating the MeshInstance3D nodes or rotating the mesh in Blender or similar.

@RobProductions
Copy link
Contributor

But what happens when an instance of the scene overrides the transform? If a user sets a scene instance's scale to (2, 2, 2), they may expect it to be 2 times as big as the scene normally is. However, if the scene was already scaled at (10, 10, 10), setting such a scale would actually make it smaller by a factor of 5.

Honestly, no, I don't know why anyone would expect it to work like that. The scale value on the instance would clearly say "10" in your example, so obviously if I set the instance scale to 2, it has decreased. Unless you mean that it was scaled to 10 in the scene root, but in that case as I just demonstrated above, that change doesn't reflect in the parent scene at all until hitting reset.

Or, if a user sets the transform to identity, they probably expect it to be unrotated, unscaled, and at the same position relative to the original scene, but this is not the behavior that happens if the scene root has a transform.

If they "reset" the transform to the default value, then yes it would reflect the root transform. But that's exactly what I would expect to happen if I hit the "reset" button. If I wanted the scene to be unrotated, unscaled, etc. I would type in 0 into the values (which would override them) because I would know that the scene defaults are different. I can see how someone that views the reset button as a "clear all" button might get confused, but that's not really how it works. But to me it's much more important that scenes and their instances have consistency - if I want something overridden (like the scale/rotation/position) I will override it and changes to the base scene will not affect them. But if I want them to match the base scene, there's currently no way to do that, and it's more confusing that these 3 properties behave differently than everything else on the root...

In those cases I would recommend either rotating the MeshInstance3D nodes or rotating the mesh in Blender or similar.

I mean yeah, we can do that, but it feels against the typical Godot workflow to have to make changes within something like a scene in order for the changes to cascade upwards. One problem is that this could artificially inflate the node count for ALL art assets/props in a level. Another issue is that this is just not how I expect it to work coming from other engines. I know this is beyond just the warning, but imo if this was fixed we wouldn't need a warning in the first place. Hope that perspective makes sense :)

@MajorMcDoom
Copy link
Contributor Author

My recommendation is that artists scale up the child nodes or scale the meshes in Blender.

Is there a basis for this recommendation? And as stated previously, the scene is authored as a default, not necessarily a neutral. It isn't practical to keep going back to Blender to scale a mesh just because I want my clouds to default to being slightly bigger/smaller.

But what happens when an instance of the scene overrides the transform? If a user sets a scene instance's scale to (2, 2, 2), they may expect it to be 2 times as big as the scene normally is. However, if the scene was already scaled at (10, 10, 10), setting such a scale would actually make it smaller by a factor of 5. Or, if a user sets the transform to identity, they probably expect it to be unrotated, unscaled, and at the same position relative to the original scene, but this is not the behavior that happens if the scene root has a transform.

I'm a little confused by this example. If you change (10, 10, 10) to (2, 2, 2), why would you expect that to be an enlargement? Are you talking about programmatically? If so, that means the developer has decided that the scale is logically meaningful at runtime, and they would be responsible for enforcing their own neutral scale on that scene. If they've determined that convention for their own scene, it doesn't really fall on the engine to enforce that for them across the whole project, and definitely not for every other Godot user.

There are also many scenes authored at non-zero rotations, like books, control panels, tools, etc.

In those cases I would recommend either rotating the MeshInstance3D nodes or rotating the mesh in Blender or similar.

That would change what the book considers as its up axis, which is not desirable. I need the book's up-axis to still be relative to the page/text, as that is a logical requirement of the code. But I need it to be lying down flat when I add a book to a scene. See here:

Image

@MajorMcDoom
Copy link
Contributor Author

@aaronfranke Glad to hear you're open to making it apply to just the position! I'd like to get a few more eyes on this issue first, just to make sure we fully understand all the different sides of this, and then if appropriate I'll open up a PR.

@aaronfranke
Copy link
Member

@RobProductions That makes sense for the editor inspector, since there is a reset button, and you can visually see what it looks like immediately, fixing any problems right then. But what about in game code at runtime? Loading a scene at runtime and running the GDScript code scale = Vector3(2, 2, 2) may have unintended effects if the scene root is scaled.

I'm a little confused by this example. If you change (10, 10, 10) to (2, 2, 2), why would you expect that to be an enlargement?

When phrased this way, it's obviously shrinking. However, it is not always the case that you have both the before and after scale visible on your screen. If you encounter a random node with a scale of (2, 2, 2), you'll see a reset button still in the editor, and there's nothing to indicate that this a fifth the default size.

Also, think about the case of rotation. Godot users expect that look_at rotates -Z to point at the target (or +Z if use_model_front is true). If a scene's default rotation is not aligned with that, it may mislead users of the scene as to which part of the model will point forward.

I usually think of it being desired to have the default transform be neutral, and the neutral transform be the default, so to me it makes sense to have a warning informing the user when this is not the case. My use cases don't have these as separate things. Maybe there's enough valid use cases to the point that this warning is undesired or should be changed to position only, but I'm unsure. However, at least allowing this warning to be disabled per project generally seems like a good idea, as we also allow this for GDScript warnings which can otherwise annoy users for valid use cases (ex: truncated integer division is a commonly desired operation, but also a common mistake).

@eon-s
Copy link
Contributor

eon-s commented Mar 18, 2025

Well, even if it feels a bit intrusive/annoying, the warning helped me to find some nodes accidentally moved, harmless in this case but that happened in the past which resulted in an incorrect offset of children when designing the scene.

Maybe the warning could be dismissed (not only this one but all the warnings), or only appear when you move the root, not when you load the scene.

Locking the root node by default (something I always do manually) could render this warning obsolete, it can be another way to prevent mistakes.

@MajorMcDoom
Copy link
Contributor Author

@aaronfranke @eon-s After reading your comments, I can see that the primary utility of this warning is to make invisible mistakes easier to catch. And I can definitely appreciate that, because those happen to me too.

However, I'd like to point out that accidental edits can happen on any property, and accidental transforms can also happen to any node (not just the root). I don't think it is quite justified to single out this one case, especially since it invalidates other existing user intents.

Also, think about the case of rotation. Godot users expect that look_at rotates -Z to point at the target (or +Z if use_model_front is true). If a scene's default rotation is not aligned with that, it may mislead users of the scene as to which part of the model will point forward.

I think that's definitely a valid concern, but that is, again, dependent on the scene and how it's used. If you are used to making games with characters, enemies, projectiles, sprites, etc. You might be very accustomed to thinking about neutral/default as being one and the same. But if you are used to making physics-based sandboxes, or games with complex objects, conventions like this really go out the window. A broom can be placed upright, so the hilt is "up". But if you can ride on it like a wizard, suddenly the hilt is "forward". Being able to assign a role of a single cardinal direction to every object, and using look_at, is a luxury that only some games can afford.

@RobProductions
Copy link
Contributor

But what about in game code at runtime? Loading a scene at runtime and running the GDScript code scale = Vector3(2, 2, 2) may have unintended effects if the scene root is scaled.

But the user was the one responsible for making the scene and writing the code in the first place, why would they be confused that setting the scale to 2 sets it to 2? Yeah, they can't "see" the scene's scale in the code, but that's exactly the same as every other property. You can't "see" the visibility of a scene that you instance in code either, yet that property cascades correctly. But you can do this:

print(visible);
print(scale);

And then you can "see" in code that the scale is different than 1. If I was programming this example and wanted the instanced scene to be 2x larger than default, I wouldn't write scale = Vector3(2, 2, 2), I would write:

scale *= 2;

So I'm not really understanding why this is a problem either...

When phrased this way, it's obviously shrinking. However, it is not always the case that you have both the before and after scale visible on your screen. If you encounter a random node with a scale of (2, 2, 2), you'll see a reset button still in the editor, and there's nothing to indicate that this a fifth the default size.

The expectation that every Scene should have a scale of (1,1,1) is just a bit odd to me, it doesn't make sense with the promise of default values and overrides. I understand that if you structure your scenes such that every scene has zeroed default values, you get certain guarantees when you use the scenes in a specific way. But someone who knows that they've (for example) scaled a scene up to 2 and wants that scale to be reflected everywhere is going to be held back by the fact that the instances ignored base scene changes. As @MajorMcDoom mentioned, this is a convention that is being forced on the user, and I'm not even talking about the warning at this point but just the way that root transforms are handled.

The fact that the Godot UI doesn't make it clear what the "default value" is could be addressed with a visibility feature. Maybe hovering over an overridden value can show what the base scene says? But that's all extra imo, what's more important is that it is consistent in the first place. If you were to view the children of a scene and mess with their scale, it would reflect properly in all instances. So why is the scene root any different? A user shouldn't be confused by this because if they want to know what the "default scale" is, they can just open up the scene and look! If they want to enforce the "all scenes should have a scale of 1" rule in their project, they're free to do so and are not hindered with this proposed change. But now that rule is kind of forced upon all developers too who are unable to modify the root the way they want.

Clarification: I've been talking mostly about scale as its the easiest example to explain but the same applies to position/rotation as well. My proposal boils down to: if the transform of an instanced scene is not overridden, it should follow the base scene just like every other property.

@aaronfranke
Copy link
Member

But the user was the one responsible for making the scene and writing the code in the first place

Not necessarily if more than one person works on a project, or someone makes assets for someone else to use, or maybe time passes and the user forgot.

Anyway, it's clear that this warning is unhelpful for many users. How about we change it to position only? However, I would leave the text as-is, so that when we do show the message for positioned nodes, it still recommends no transforming.

@Mickeon
Copy link
Contributor

Mickeon commented Mar 18, 2025

We had to remove the annoying z_index warning on Control for similar reasons.
I also think it would be best to tone this warning down to only interest position. It's the most likely to be a "mistake".

Just for a bit of clarity, we do not allow warnings to be dismissed because it makes users complacent, instead of aware that what they're doing is unconventional.
See also:

@aaronfranke
Copy link
Member

I've opened a PR for this: #104331

@Mickeon
Copy link
Contributor

Mickeon commented Mar 19, 2025

Shouldn't this be closed by #104331 ?

@KoBeWi
Copy link
Member

KoBeWi commented Mar 19, 2025

Yes, the title specifically mentions scaled roots.
I suppose this was left open for discussion, but I'd assume the PR resolved it and see whether it pops up again.

@KoBeWi KoBeWi closed this as completed Mar 19, 2025
@KoBeWi KoBeWi added this to the 4.5 milestone Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants