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

Fix the Python type error when creating the .sln file #75309

Merged
merged 1 commit into from
Apr 5, 2023

Conversation

HK-SHAO
Copy link
Contributor

@HK-SHAO HK-SHAO commented Mar 25, 2023

Fix: #74725

The error occurs at the statement env["CPPDEFINES"] + defines , where env["CPPDEFINES"] is of type <class 'collections.deque'> and defines is of type <class 'list'> .

Since the two types cannot be added together, list() function is used to convert deque to list.

After the modification, it can be compiled and generate the .sln file normally.

@HK-SHAO HK-SHAO requested a review from a team as a code owner March 25, 2023 07:12
@RedworkDE
Copy link
Member

Does this fix also work with scons 4.4.0? (downgrading to that is the current workaround in #74725) It probably should, just checking to ensure this fix doesn't break currently working setups / when or if scons fixes this on their end.

@HK-SHAO
Copy link
Contributor Author

HK-SHAO commented Mar 25, 2023

Does this fix also work with scons 4.4.0? (downgrading to that is the current workaround in #74725) It probably should, just checking to ensure this fix doesn't break currently working setups / when or if scons fixes this on their end.

This can solve the problem.

I compiled and generated .sln successfully in both v4.4.0.fc8d0ec215ee6cba8bc158ad40c099be0b598297 and v4.5.2.120fd4f633e9ef3cafbc0fec35306d7555ffd1db .

@YuriSizov YuriSizov added this to the 4.1 milestone Mar 25, 2023
@Calinou Calinou added cherrypick:4.0 cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Mar 27, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine for now as a workaround.

I see the SCons devs are discussing further changing CPPDEFINES to make sure it's properly coerced into list whenever it's being accessed, regardless of whether it's a list or deque internally.

So hopefully in a future 4.5.x or 4.6 release this workaround shouldn't be needed anymore. But it doesn't hurt to have it until then, and for some time to keep 4.5.0-4.5.2 working.

@akien-mga akien-mga merged commit 6935216 into godotengine:master Apr 5, 2023
@akien-mga
Copy link
Member

akien-mga commented Apr 5, 2023

Thanks! And congrats for your first merged Godot contribution 🎉

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.3.

@ggsjyoon
Copy link

The fix worked with SCons v4.5.2. Thanks!

@akien-mga akien-mga changed the title Fix the Python type error when creating the .sln file Fix the Python type error when creating the .sln file May 12, 2023
@akien-mga
Copy link
Member

Cherry-picked for 3.6.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Aug 28, 2023
@akien-mga
Copy link
Member

Cherry-picked for 3.5.3.

@akien-mga akien-mga removed the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError in methods.py when running initial build with vsproj=yes (SCons 4.5.0 regression)
6 participants