Ammo and reloading #28
Conversation
There was a problem hiding this comment.
You really want to avoid adding boolean variables. This is the most important feedback I have in this PR.
Every bit of state and code that you add increases the surface area for bugs. Especially the state because it's so easy to forget to switch a boolean.
Here's the code changes I made to remove the booleans, but also group the input into a single function. I recommend you to study it, you'll see it removes over 20% of the code size, meaning once you get used to doing this, it will save you time coding on top of preventing more bugs (thus saving you maintenance work): b3fc5b2#diff-91075a352ce78af7185f36825fafb6413e9c884a31de05e23b114cd5f75bf2d0
| var is_reloading := false | ||
| var left_click_held := false | ||
| var shoot_ready := true |
There was a problem hiding this comment.
One of the main sources of bugs in computer programs is the addition of unnecessary state. Try to use existing state over adding new variables whenever you can. Here, you can use the timers and the Input object in _process() to replace these 3 extra variables.
There was a problem hiding this comment.
Last tasks:
- Move the code that gets and connects sliders from Player.gd to AmmoReloadingDemo.gd (because this UI isn't part of the Player scene, the corresponding code shouldn't be in the Player scene)
- Move the player-specific UI (the reloading bar and ammo display) into the player scene. You can add a CanvasLayer in the Player scene to ensure the UI always displays in front
There was a problem hiding this comment.
The code and features are good, good job on that. there are two small issues left:
- The interface covers the player's weapon.
- This pull request changes the demos' main scene. Wouldn't this affect the ranged attack video tutorial, where viewers can assume the demo opens on the scene where you can shoot arrows?
You can see issue 1. here:
|
Merged manually as a02951f |
Please check if the PR fulfills these requirements:
Related issue (if applicable): #
What kind of change does this PR introduce?
Added the ammo and reloading demo to the ranged attacks folder
Does this PR introduce a breaking change?
New feature or change
What is the current behavior?
What is the new behavior?
A demo with basic bullet shooting with an ammo and reload system.
Other information