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

2D movement overview - Click-and-move - use actions #9835

Merged
merged 8 commits into from
Sep 5, 2024

Conversation

HubbleCommand
Copy link
Contributor

Changed the code sample for the Click-and-move section of the 2D movement overview page to use Actions instead of the InputEvent, to match the sample.

Changed it to use actions instead of the event, like in the sample
tutorials/2d/2d_movement.rst Outdated Show resolved Hide resolved
tutorials/2d/2d_movement.rst Outdated Show resolved Hide resolved
@AThousandShips AThousandShips added area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:2d labels Aug 25, 2024
@AThousandShips
Copy link
Member

The C# example should be updated accordingly

HubbleCommand and others added 2 commits August 25, 2024 17:42
Co-authored-by: A Thousand Ships <[email protected]>
@HubbleCommand
Copy link
Contributor Author

The C# example should be updated accordingly

I don't know if there is one? I can't see any mono in godot-docs-project-starters and there is no 2d movement starter in the mono demos.

@AThousandShips
Copy link
Member

The example in the code right below:

public override void _Input(InputEvent @event)
{
if (@event is InputEventMouseButton eventMouseButton)
{
if (eventMouseButton.ButtonIndex == MouseButton.Left && eventMouseButton.Pressed)
{
_target = GetGlobalMousePosition();
}
}
}

@HubbleCommand
Copy link
Contributor Author

HubbleCommand commented Aug 25, 2024

The example in the code right below:

public override void _Input(InputEvent @event)
{
if (@event is InputEventMouseButton eventMouseButton)
{
if (eventMouseButton.ButtonIndex == MouseButton.Left && eventMouseButton.Pressed)
{
_target = GetGlobalMousePosition();
}
}
}

ah, I am blind, done!

@AThousandShips AThousandShips requested a review from a team August 26, 2024 10:57
tutorials/2d/2d_movement.rst Outdated Show resolved Hide resolved
HubbleCommand and others added 2 commits August 26, 2024 13:28
Co-authored-by: A Thousand Ships <[email protected]>
Co-authored-by: Raul Santos <[email protected]>
@skyace65
Copy link
Contributor

We updated this code snippet 3 months ago in #9427 which is why it doesn't line up with what's in the demo project repository, it wasn't updated there. I need someone to explain to me why one method of getting input is better than the other, or better practice, before this can be merged, because I've just tested the old and new code and both work.

@HubbleCommand
Copy link
Contributor Author

HubbleCommand commented Aug 27, 2024

We updated this code snippet 3 months ago in #9427 which is why it doesn't line up with what's in the demo project repository, it wasn't updated there. I need someone to explain to me why one method of getting input is better than the other, or better practice, before this can be merged, because I've just tested the old and new code and both work.

From the documentation:

However, it is cleaner and more flexible to use the provided :ref:InputMap <class_InputMap> feature, which allows you to define input actions and assign them different keys

@HubbleCommand
Copy link
Contributor Author

I don't know why #9197 says the previous InputMapping example didn't work. #9197 says it doesn't work in 4.2.1.stable. I just tested it and it does, using either the passed event variable or the Input global. It also works in 4.3.

The other parts of this page also use Actions with get_vector & get_axis in GDScript and C#, so I don't know why only that one was changed? It's also what's still in the start project, which still works.

@skyace65 skyace65 merged commit 8a922af into godotengine:master Sep 5, 2024
1 check passed
@skyace65
Copy link
Contributor

skyace65 commented Sep 5, 2024

Thanks! And congrats on your first merged PR!

mhilbrunner pushed a commit that referenced this pull request Oct 4, 2024
2D movement overview - Click-and-move - use actions
@mhilbrunner
Copy link
Member

Cherry-picked to 4.3 in #10038.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:2d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants