macalimlim / godot-rust-template

A template for godot-rust projects
MIT License
51 stars 22 forks source link

separate godot and rust project directories #20

Closed macalimlim closed 3 years ago

macalimlim commented 4 years ago

separate godot and rust project directories, resolves #16

macalimlim commented 3 years ago

godot/native/game.gdnlib and godot/native/Main.gdns should be removed from the tree, since these files would be generated by project-utils.

I think its better to have the gdnlib and gdns files after the project has been generated by the template. The project-utils should ignore these files if they are already existing which is already done in this commit

I'm really not sure if I'd want to build for every platform on build-debug or build-release. Even for export, I'd prefer it if the mobile platforms are opt-in and commented out by default, since these platforms are not always applicable, and users might not always have the necessary SDKs installed. These introduce friction that prevent the template from serving its purpose.

Sure I can comment out the make commands initially and let the user un-comment which lines he/she want to be built/exported. I was thinking the other way around :sweat_smile:

ghost commented 3 years ago

I think its better to have the gdnlib and gdns files after the project has been generated by the template.

Is there a way to make the name of game.gdnlib dependent on the name of the crate? Because for a project named foo, project-utils would name the file foo.gdnlib. It would be great if this behavior can be made consistent.

macalimlim commented 3 years ago

I think its better to have the gdnlib and gdns files after the project has been generated by the template.

Is there a way to make the name of game.gdnlib dependent on the name of the crate? Because for a project named foo, project-utils would name the file foo.gdnlib. It would be great if this behavior can be made consistent.

Unfortunately, the current version of cargo-generate does not support this, but it was supported in an earlier version. I'm not sure why was it removed. I filed an issue asking about this...

ghost commented 3 years ago

I see. I think we can make a separate issue tracking the upstream one. In that case this seems good to merge to me for now!