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

C#: Make editor create NuGet fallback folder for Godot packages #43029

Merged
merged 1 commit into from
Oct 27, 2020

Conversation

neikeq
Copy link
Contributor

@neikeq neikeq commented Oct 23, 2020

Main benefits:

  • Projects can be built offline. Previously you needed internet access the first time building to download the Godot packages (Godot.NET.Sdk).
  • Changes to packages like Godot.NET.Sdk can be easily tested before publishing. This was already possible but required too many manual steps.
  • First time builds are a bit faster, as the Sdk package doesn't need to be downloaded. In practice, the package is very small so it makes little difference.

Bumped Godot.NET.Sdk to 4.0.0-dev3 in order to enable the recent changes regarding .mono/ -> .godot/mono/.

@neikeq
Copy link
Contributor Author

neikeq commented Oct 23, 2020

This will be backported to the 3.2 branch later, but we probably want to give it a bit more time to be tested.

@akien-mga
Copy link
Member

#43028 is merged, git rebase upstream master should remove the now merged commit from this PR.

@neikeq
Copy link
Contributor Author

neikeq commented Oct 23, 2020

#43028 is merged, git rebase upstream master should remove the now merged commit from this PR.

I was crossing my fingers in hope GitHub would magically take care of that 😄

@neikeq
Copy link
Contributor Author

neikeq commented Oct 23, 2020

Looks like I have to update my pre-commit hooks... :(

@aaronfranke
Copy link
Member

.godot/.mono/

Note: It's actually .godot/mono, we don't need to hide the folder twice.

@neikeq
Copy link
Contributor Author

neikeq commented Oct 23, 2020

@akien-mga Are the CI static checks different from the pre-commit hooks in any way? Mine don't trigger any error. Perhaps it's my formatter version:

~ $ black --version
black, version 19.3b0

@akien-mga
Copy link
Member

@neikeq That one is ran manually by the CI: misc/scripts/file_format.sh
It's not part of pre-commit hooks because it's slow as it parses all repo files instead of the ones changed by the commit, like the pre-commit hooks do.

@aaronfranke
Copy link
Member

Here the failing check is "Python style checks via black (black_format.sh)"

Black is pinned to 20.8b1: https://github.com/godotengine/godot/blob/master/.github/workflows/static_checks.yml#L22

@akien-mga
Copy link
Member

Ah yes, black changes its mind on how to reflow long strings all the time, so it's important to use the current stable version which we're tracking (in the past we used master as there was no recent release and there was an important bugfix we needed, but now it's in 20.8b1).

Main benefits:
- Projects can be built offline. Previously you needed internet
  access the first time building to download the packages.
- Changes to packages like Godot.NET.Sdk can be easily tested
  before publishing. This was already possible but required
  too many manual steps.
- First time builds are a bit faster, as the Sdk package doesn't
  need to be downloaded. In practice, the package is very small
  so it makes little difference.

Bumped Godot.NET.Sdk to 4.0.0-dev3 in order to enable the
recent changes regarding '.mono/' -> '.godot/mono/'.
@aaronfranke
Copy link
Member

aaronfranke commented Oct 24, 2020

I did a little testing. In the current master, building fails due to the mismatch of .mono/ and .godot/mono/. With this PR, pressing the build button causes the editor to crash, but the good news is that it didn't create the .mono/ folder :P

Anyway, here's a minimal test project, including .godot/mono/ and the logs I get when building: Test.zip For reference I'm on Ubuntu 20.04 with Mono 6.12.0.90 and .NET Core 3.1.403. I also had to copy over the csproj and sln files from a Godot 3 project and modify them, because I'm not able to open the Create Script menu without freezing my entire computer. Oh and also here's the terminal output: terminal-output.txt

@neikeq
Copy link
Contributor Author

neikeq commented Oct 26, 2020

@aaronfranke Thanks. Custom event signals weren't doing hot-reload properly. #43088 fixes it.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Tested in combination with #43088 and it works. I don't understand all this code but it looks fine.

Note: I didn't test offline builds, but at least the online builds work now unlike the current master.

@akien-mga akien-mga merged commit c11992e into godotengine:master Oct 27, 2020
@akien-mga
Copy link
Member

Thanks!

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.

3 participants