search

godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.75k stars 577 forks source link

Move tools to site_scons/site_tools, which is the canonical scons tools directory. Pass env["toolpath"] down the tool chain in godotcpp.py. #1645

Open Ivorforce opened 2 days ago

Ivorforce commented 2 days ago

We should strive to adapt tool standards.

With this change, passing toolpath is no longer required in normal cases. Read more here: https://scons.org/doc/production/HTML/scons-user/ch07s04.html#id1354

Ivorforce commented 2 days ago

Ok, I also solved one issue where toolpath wasn't passed down as is normally expected through godotcpp.py and SConstruct.

I hope the new implementation is now more idiomatic. One reason I changed it is because I was running into issues trying to use custom tool and working directories, and the current implementation made it impossible because of path assumptions.

dsnopek commented 2 days ago

I don't know about this.

The site_scons/ directory feels more like a thing for a full project using scons (which in this case would be the GDExtension project that uses godot-cpp) rather than godot-cpp itself, which is more of a "component" used by other projects.

Also, setting a custom toolpath is fairly normal thing to do. I don't see our use of that feature being particularly hacky or anything.

One reason I changed it is because I was running into issues trying to use custom tool and working directories, and the current implementation made it impossible because of path assumptions.

Can you explain in more detail the problem you were having? Perhaps there is a simpler solution.

Ivorforce commented 1 day ago

Can you explain in more detail the problem you were having? Perhaps there is a simpler solution.

There's a bit of a background to this PR. I opened https://github.com/godotengine/godot-cpp/issues/1646 to let you in on my thoughts.