added Third-Person-Shooter example

[idanmuze]

This PR is a port of the character controller from the official Godot TPS demo. The project currently utilizes a modified GDScript _physics_process function, unlike the Rust present for most of the project. The decision was a temporary fix to a very subtle character movement/rotation bug that I could not figure out. The somewhat working, intended Rust implementation is in the src folder under the name _physics_process_wip. I urge anyone with a solution or an explanation to let me know/open a PR so we can replace the GDScript ASAP.

[chitoyuu]

129 files and 20MB of data seem to be a bit too much compared to the scale of the other examples. Would it make sense to leave out the assets and instead include some instructions on how to acquire them from official sources?

Alternatively, remove the parts irrelevant to the topic here (the character controller), such as the level geometry, camera behavior, and excessive graphic effects.

a very subtle character movement/rotation bug that I could not figure out

It would help greatly if you would go into some details here. What's the expected behavior? What did you actually get? Perhaps a side-by-side comparison clip?

Also, since this PR isn't ready for merging yet, consider converting it to a draft PR.

[Bromeon]

Thanks for the big effort! 🙂

Given the scale of this example, I wonder if it wouldn't be better suited to live outside the main repository, e.g. referenced by the book. Asset size is just one aspect, but also the complexity and configuration of resources and the scene tree is rather high.

The question is of course also: who is going to maintain such a big example, if Godot evolves and breaks some of the things 😉

[idanmuze]

Thanks for the detailed feedback. Would downsizing the project to the core character controller and writing a concise guide to acquire the assets rather than including them work?

That would leave the problem of maintenance, which I'm not sure how to answer. Would the previously mentioned simplifications be enough to make it maintenance friendly?

[idanmuze]

a very subtle character movement/rotation bug that I could not figure out

It would help greatly if you would go into some details here. What's the expected behavior? What did you actually get? Perhaps a side-by-side comparison clip?

@chitoyuu
Does this work?

GodotBugDemoSmall.mp4

[idanmuze]

Got the file size down to 205kb and squashed the bugs! Where do we go from here? @chitoyuu

[chitoyuu]

This looks a lot better just from the file/line count. I don't think it's nearly as big a liability to maintain as it was originally. Of course we have to see what the maintainer thinks, but here's a few quick suggestions to further simplify the code.

[chitoyuu]

Things are looking better, although there are still some concerns.

Note that you do not have to co-author me in every single commit where you applied my suggestions. Nothing I've said here is super substantial -- just what's in the docs and common idioms of the language. You'll also need to rebase and squash most of the commits anyway, since we want a clean commit history.

[idanmuze]

Things are looking better, although there are still some concerns.

Note that you do not have to co-author me in every single commit where you applied my suggestions. Nothing I've said here is super substantial -- just what's in the docs and common idioms of the language. You'll also need to rebase and squash most of the commits anyway, since we want a clean commit history.

I'll make sure to now only co-author on things that don't come from the consensus of the language.

Is it better to make granular commits and then rebase them, or do developers make clean, larger commits right off the bat? Are my commits too granular?

[chitoyuu]

Is it better to make granular commits and then rebase them, or do developers make clean, larger commits right off the bat? Are my commits too granular?

That's up to you. Here what matters is the end result: that the commit history of master be linear and easy to follow for maintenance. It doesn't matter what it took to get there.

Personally I'd prefer if you rebase your code as you go, because that allows me to merge the PR immediately when it becomes ready without having to wait for you to rebase.

[chitoyuu]

Ok, this should be the last of necessary changes. I think this is a good demonstration of what GDScript-coding looks like in Rust. It provides a good reference point for future QoL improvements aimed at easing engine interaction, which dodge-the-creeps for its simplicity doesn't do extremely well.

Once all the issues are solved and CI passes, rebase again and we should be good to go.

Concerns unrelated to specific files

• The example should have a .gitignore file for the player/ directory that users are supposed to obtain themselves.
• When opening Player.tcsn, the console is flooded with the following error messages:

ERROR: Index p_point = 5 is out of bounds (blend_points_used = 5).
   at: get_blend_point_node (scene/animation/animation_blend_space_2d.cpp:110)
ERROR: Index p_point = 6 is out of bounds (blend_points_used = 5).
   at: get_blend_point_node (scene/animation/animation_blend_space_2d.cpp:110)
ERROR: Index p_point = 7 is out of bounds (blend_points_used = 5).
   at: get_blend_point_node (scene/animation/animation_blend_space_2d.cpp:110)
...

I have confirmed that this also happens with the original GDScript project, but it could be mentioned somewhere in the README that this is to be expected.

---

bors try

[bors]

try

Build failed:

• full-ci

[idanmuze]

Wow! I picked up a lot from this process. Glad I had you as my reviewer @chitoyuu. @Bromeon as well.

[chitoyuu]

The code should be fine now! There are still details that I think aren't exactly ideal, but those can be sorted out over time, and there's no point in delaying this further over such minor issues.

However, I can't merge this yet because you still have a merge commit in your history. Please rebase your branch on master such that only the 2 following commits remain:

• Added refactored TPS Example
• Implemented MulAssign for Transform

Force-push the branch, and then we should be good to go!

bors try

[bors]

try

Build succeeded:

• full-ci

[chitoyuu]

Honestly I can't say I'm sure about the co-authoring, but I see no point in delaying the PR further over this so I'll take it.

bors r+

[bors]

Build succeeded:

• full-ci

[idanmuze]

Honestly I can't say I'm sure about the co-authoring, but I see no point in delaying the PR further over this so I'll take it.

Do you have any pointers on when co-authoring is more appropriate?

[chitoyuu]

Personally I find it appropriate when the other person put in a substantial portion of the total effort, like when:

• If they did in fact write a lot of the code, most obviously. This applies when building upon someone else's PR, or incorporating someone else's patch (LoC is a bad metric, but let's say at least a few dozen lines or so).
• If they helped extensively to debug an issue that is hard to reproduce, e.g. due to lack of platform access.
• If they otherwise provided crucial input during the development.

Use discretion. Is the other person, give or take, as important to the commit as you? If so, co-author them. If not, don't.