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

BoxShape3D and RectangleShape2D has incorrect size after project conversion due to change how size/extents are treated in 4.x #77931

Closed
Tracked by #73960
sketchyfun opened this issue Jun 6, 2023 · 17 comments · Fixed by #82754

Comments

@sketchyfun
Copy link
Contributor

Godot version

v4.1.dev.custom_build [0a0132c]

System information

Windows 10

Issue description

When converting a project from 3 to 4, any BoxShape3D resources will end up being half the size they were in Godot 3.x

This seems to be due to the fact BoxShape3D's dimensions were defined by half extents, and now Godot 4.x uses full extents.

So a BoxShape3D that had extents of 1,1,1 in Godot 3 was actually 2m x 2m x 2m, whereas now in Godot 4 those same values would be treated as 1m x 1m x 1m

Steps to reproduce

Create a Godot 3.x project with a BoxShape3D shape as part of a collider
Convert the project to 4.x using the project converter and open it
Notice that the BoxShape3D now appears to be half the size in the editor despite the values being the same

Minimal reproduction project

N/A

@KoBeWi
Copy link
Member

KoBeWi commented Jun 6, 2023

BoxShape3D already handles extents property, so this conversion should be removed from the converter. Also might affect LightmapGI and ReflectionProbe.

@sketchyfun
Copy link
Contributor Author

BoxShape3D already handles extents property, so this conversion should be removed from the converter. Also might affect LightmapGI and ReflectionProbe.

Are you saying BoxShape3D already deals with the old extents property and converts it into the new size property? Sorry, bit confused by what you mean

@KoBeWi
Copy link
Member

KoBeWi commented Jun 6, 2023

Yes

bool BoxShape3D::_set(const StringName &p_name, const Variant &p_value) {
if (p_name == "extents") { // Compatibility with Godot 3.x.
// Convert to `size`, twice as big.
set_size((Vector3)p_value * 2);
return true;
}

@sketchyfun
Copy link
Contributor Author

sketchyfun commented Jun 7, 2023

Ah okay. So the issue is with BoxShape3D doing the conversion and the project converter doing it as well? I converted my project a while ago, so it may be that it wasn't handled by the converter back then, or maybe some other weirdness happened. Either way the size of all of them was half what it should be and I ended up writing some regex stuff to go through and fix all 300+ instances in my project

@ARez2
Copy link
Contributor

ARez2 commented Jun 9, 2023

Hmm I looked into project_converter_3_to_4.cpp but I can't find the place where the extents conversion happens...

@KoBeWi
Copy link
Member

KoBeWi commented Jun 9, 2023

It's in another file

{ "extents", "size" }, // BoxShape3D, LightmapGI, ReflectionProbe

@ARez2
Copy link
Contributor

ARez2 commented Jun 9, 2023

Yes I found that as well. A really weird thing just happened. So I used my local version of Godot (did git pull before, v4.1.dev.custom_build.0a0132ccf) and converted this project:
Conversion3.zip

Its just a simple scene with a CSGBox3D. Weird thing is: In Godot 3.5 (Steam version), the size was (2, 4, 6) but after converting it to Godot 4 using my engine version (I changed nothing in the converter files), the size was (1, 4, 6)... Can anyone explain why that would happen?

@ARez2
Copy link
Contributor

ARez2 commented Jun 9, 2023

Sorry I forgot to test out BoxShape3D. So, updated project:
Conversion3.zip

Converting this project ("Convert full project") also produces that (1, 4, 6) size on the CSG node, but leaves the newly added CollisionShape3D with a BoxShape of size (2, 4, 6) at that exact size. So does the conversion not work?

Can someone else try this out?

@KoBeWi
Copy link
Member

KoBeWi commented Jun 9, 2023

Seems like 3.x CSGBox had width/height/depth properties instead of size. In your scene:

[node name="CSGBox" type="CSGBox" parent="."]
height = 4.0
depth = 6.0

It's missing width (default 2), but the default x size in 4.x is 1.

@Zireael07
Copy link
Contributor

@KoBeWi 90% sure this is the case but I can't check as my CSG-using project is broken by an attempt to convert to 4.x

@elchc
Copy link

elchc commented Sep 14, 2023

Can confirm that adding width to the G3 project's Main.tscn file gives you this when you convert it:

[node name="CSGBox3D" type="CSGBox3D" parent="."]
width = 2.0
height = 4.0
depth = 6.0

And gives you a box sized (2, 4, 6). I never used 3.x so I'm unclear whether it omits dimensions if the values are default size.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 14, 2023

Default values are not stored.

@elchc
Copy link

elchc commented Sep 16, 2023

So a fix for this would either be 1) something that adds a line width/height/depth = 2.0 if no value is found, OR 2) something that changes width/height/depth to size = Vector3(w,h,d) format. I'm going to try option 2.

@BrunoArmondBraga
Copy link
Contributor

Hii everyone, I am working on this issue! Just trying to understand the changes needed on LightmapGI so I can safely delete the conversion from the renames_map_3_to_4.cpp file

@elvisish
Copy link

Can this be changed to include CollisionShape2D? (Also is imported at half the size due to extents changing)

@Calinou Calinou changed the title BoxShape3D has incorrect size after project conversion due to change how size/extents are treated in 4.x BoxShape3D and RectangleShape2D has incorrect size after project conversion due to change how size/extents are treated in 4.x Sep 26, 2023
@BrunoArmondBraga
Copy link
Contributor

BrunoArmondBraga commented Sep 30, 2023

Trying to solve the problem, I commented the line

{ "extents", "size" }, // BoxShape3D, LightmapGI, ReflectionProbe

But this arrived a new issue: when trying to access your own extents property (on a ReflectionProbe converted script, for example), Godot 4 rises the error Identifier 'extents' not declared in the current scope. However, if you use self.extents it works fine. So, removing the extents rename causes this type of script error as a side effect.
Should I create a PR to solve this issue and open a new issue to deal with this side effect? Or should we try to solve everything at once?

@KoBeWi
Copy link
Member

KoBeWi commented Oct 1, 2023

I think script errors are ok, because you can see that something is wrong. In case of scenes the values are silently lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants