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

Add nix flake for developers #4960

Merged
merged 12 commits into from
Apr 1, 2024
Merged

Add nix flake for developers #4960

merged 12 commits into from
Apr 1, 2024

Conversation

ottoblep
Copy link
Contributor

@ottoblep ottoblep commented Mar 23, 2024

Brief Description of What This PR Does

Since i recently switched to NixOS, it was necessary to define all the the tools required for Thrive development into a file.
For any developers using the nix package manager or NixOS this will make the process of setting up a development environment easier.
So far this does not include a derivation to fully build a Thrive release (which is theoretically possible), but just a shell environment that installs all programs required for following setup_instructions.md.
I am aware that having these files adds additional maintenance costs, each time the build process changes.
As an alternative I could keep the definitions on my account and we could add a small note for nix users in setup_instructions.md.
Feel free to add your suggestions on weather this makes sense to have in the main repo.

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

  • PR author has checked that this PR works as intended and doesn't
    break existing features:
    https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
    (this is important as to not waste the time of Thrive team
    members reviewing this PR)
  • Initial code review passed (this and further items should not be checked by the PR author)
  • Functionality is confirmed working by another person (see above checklist link)
  • Final code review is passed and code conforms to the
    styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

@hhyyrylainen
Copy link
Member

I am aware that having these files adds additional maintenance costs, each time the build process changes.
As an alternative I could keep the definitions on my account and we could add a small note for nix users in

I'm fine with them being in the main repo as long as we can manage expectations that the nix file might not always be up to date and if someone runs into a problem they should send an update PR.

For the full build, this probably prevents making full thrive builds right now: godotengine/godot#89674

@ottoblep
Copy link
Contributor Author

I will work with the environment a bit and see how it goes, before setting this PR as ready.

@ottoblep
Copy link
Contributor Author

So far i didn't manage to get the debugger working with vscode, which is pretty important to me. I will try some more configurations.

@hhyyrylainen
Copy link
Member

For Godot 4 Rider's debugger only half works: it seems like the game crashes on exit 100% of the time when using it.

@GameDungeon
Copy link
Contributor

GameDungeon commented Mar 29, 2024

I was considering doing this myself, but I just found this PR. It might be worth making a real nix-pkg for godot4-mono given it's currently missing, but that is up to you.

Now that I swapped to nix there will be at least one other person to maintain the build process, so that is good.

Edit: The debugger not working is just as likely a godot4-mono issue as it is a nix issue. The ecosystem hasn't caught up yet, so a lot of stuff is currently broken still

Edit2: It actually seems a PR for godot4-mono is open. Before I only saw the closed PR. If you run unstable it might be best to swap to that given it should be merged soon. NixOS/nixpkgs#285941

@ottoblep
Copy link
Contributor Author

Thanks for the tip @GameDungeon. Glad somebody else can benefit. I switched godot to the derivation from the nixpkgs PR. Since I use 23.11 I kept the rest of the packages from there. But you can easily override the input to unstable if you prefer.

@ottoblep
Copy link
Contributor Author

I'm able to pause the execution in the godot editor but can't manage to trigger a breakpoint. Weird.

@hhyyrylainen
Copy link
Member

I'm able to pause the execution in the godot editor but can't manage to trigger a breakpoint. Weird.

Did that work for C#? I'm asking as I've never used that. I've always used an external debugger for Godot and the move to 4 didn't really impact me (other than needing to select different run configurations as those are now different in Rider compared to Godot 3).

@GameDungeon
Copy link
Contributor

GameDungeon commented Mar 31, 2024

In the Godot Editor I'm pretty sure breakpoints never worked for c#. At least they never did for me. I needed to get a vscode extension to get them working at all. I'm pretty sure said extension is currently broken for godot 4 mono too, but there could be others.

Edit: Looking online there are fixes for various editors for c# and godot 4, but the built-in editor has never supported breakpoints with C#.

@hhyyrylainen
Copy link
Member

Edit: Looking online there are fixes for various editors for c# and godot 4, but the built-in editor has never supported breakpoints with C#.

I use Rider with the Godot plugin (https://github.com/JetBrains/godot-support). And it has just worked. Just letting people know what I use and how it seems to be well supported.

@ottoblep
Copy link
Contributor Author

ottoblep commented Mar 31, 2024

I was a bit quick to assume the nix environment is at fault. As you said the vscode extensions for c# debugging are in various stages of updating to godot4. For this PR that's not really relevant.
Edit: If you have any suggestions what else would be useful in the flake let me know, otherwise this should be ready.

@ottoblep ottoblep marked this pull request as ready for review March 31, 2024 20:07
@hhyyrylainen hhyyrylainen added this to the Release 0.6.6 milestone Mar 31, 2024
@ottoblep
Copy link
Contributor Author

I will pin some more of the tools to the exact version the CI uses.

@GameDungeon
Copy link
Contributor

LGTM. Code all looks good, and I can verify that it all works on my system. Thanks for the good work :)

We will need to update the nix after the godot pr is merged, but I see no reason that should hold this back.

@hhyyrylainen hhyyrylainen merged commit 48ec670 into master Apr 1, 2024
4 checks passed
@hhyyrylainen hhyyrylainen deleted the nix_dev_env branch April 1, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants