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

Updated Dodge the Creeps C# to Godot mono 4.2 #1000

Conversation

captain-redbeard
Copy link
Contributor

@captain-redbeard captain-redbeard commented Dec 13, 2023

This change updates the C# Dodge the Creeps project to Godot 4.2 based on the updated documentation from here: https://docs.godotengine.org/en/stable/getting_started/first_2d_game/index.html

Version: Godot mono 4.2

Changes

- Scene files changed to lowercase.

  • Switched to explicit types as the documentation mentions that this should normally be used. (Needs documentation update to match code)
    - Removed particle effect as it's not in the documentation.
  • Removed GD.Randomize() from Main.cs as it's not in the documentation.
  • Reuse ShowMessage() method in HUD.cs. (Needs documentation update)

Possible issues

- Godot's editor defaults to tab indention, opening this project and saving any CS file will result in the indention being converted to tabs.
- Should we switch to tab indention to avoid this?

@AThousandShips
Copy link
Member

@Calinou
Copy link
Member

Calinou commented Mar 5, 2024

Sorry for the delay on this. Could you look into rebasing this PR against the latest master branch so we can merge this?

  • Godot's editor defaults to tab indention, opening this project and saving any CS file will result in the indention being converted to tabs.
    • Should we switch to tab indention to avoid this?

Yes, I'd switch all C# files to use tabs. The docs always use spaces because we have to use spaces for indentation there, even in GDScript.

  • Removed particle effect as it's not in the documentation.

The particle version is present in the GDScript version of the demo as well, so I'd prefer to keep it for consistency. We should look into amending the documentation eventually.

}

public void NewGame()
private void NewGame()
{
// Note that for calling Godot-provided methods with strings,
// we have to use the original Godot snake_case name.
Copy link
Member

Choose a reason for hiding this comment

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

This comment should be updated to talk about using the names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment should be updated to talk about using the names

I'm not sure what you mean?

@Calinou
Copy link
Member

Calinou commented Mar 24, 2024

@captain-redbeard Bump 🙂

@captain-redbeard captain-redbeard force-pushed the feature/4-2-upgrade-mono-dodge-the-creeps branch from 05dd37e to 5c5e7a6 Compare April 3, 2024 11:04
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.

Thank you for your work on this! I spent a few hours resolving the differences between this PR and #870, ensuring best practices, no regressions, and consistency with the tutorial in the documentation.

Ultimately what I ended up with was quite different from both PRs. I have pushed the consolidations to both PRs so that you can view them, but I can only accept one PR. As an arbitrary choice, I will be accepting this PR and closing PR #870. I will squash-and-merge and include @van800 as a co-author.

@aaronfranke aaronfranke merged commit fbef18f into godotengine:master Apr 12, 2024
1 check passed
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.

4 participants