-
-
Notifications
You must be signed in to change notification settings - Fork 478
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
Implement sprinting mechanics #4801
base: master
Are you sure you want to change the base?
Conversation
to make sure it's always running, also add a hotkey in the ability bar for sprinting
79b49ca
to
6adb998
Compare
and add support for excess strain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're settled on the basic implementation idea then I think it's good to get this into a BOTD soon so that we can get player feedback on how it feels.
@@ -141,7 +141,7 @@ protected override void Update(float delta, in Entity entity) | |||
/// </summary> | |||
private void ApplyATPDamage(CompoundBag compounds, ref Health health, ref CellProperties cellProperties) | |||
{ | |||
if (compounds.GetCompoundAmount(atp) > 0) | |||
if (compounds.GetCompoundAmount(atp) > 0.1f) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed? I think this may have massive implications on taking ATP damage when trying to move.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was changed as ATP never reached a low enough level for it to do damage. The bar would look empty but no damage would be done. Should've probably set it to something lower though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjusting when the ATP is taken would be a better choice. That's what's been done in the past to ensure cells can be damaged (for example making this run before whatever system applies ATP damage, but after the process system).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up changing this to 0.05f
, as I couldn't get this to work otherwise. I think it's sensible to set it to that, as the difference at that point is barely noticeable and I don't think it would have any major implications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment before this if to explain the number? Or actually I think it'd be pretty nice to make it a constant in Constants and add a summary comment explaining the reason for it there.
I think mechanically, this feature is mostly ready. But since it still needs some work maybe it should be classed as an "experimental feature"? To recap, what is needed is balance, AI implementation, a tutorial and maybe some graphics improvements. |
I suppose without those still undone parts classifying this as an experimental feature would be an option to merge this soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a full code review. I don't see any major issues anymore (unless some pop up when I playtest this), but there's quite many small details that can be tweaked to make this feature more maintainable and improve code clarity, and maybe fix one small bug.
This should be ready for merge now. I've fixed everything from the review. |
@@ -311,6 +317,18 @@ protected override void UpdateHealth(float delta) | |||
hpLabel.HintTooltip = hpText; | |||
} | |||
|
|||
protected override float? ReadPlayerStrainFraction() | |||
{ | |||
if (stage!.Player.Has<StrainAffected>() && !playerMissingStrainAffected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think first condition should be with ! at the beggining
@@ -53,6 +53,11 @@ public struct MicrobeControl | |||
/// </summary> | |||
public bool SlowedBySlime; | |||
|
|||
/// <summary> | |||
/// Whether this microbe is currenyly sprinting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo
We are currently in feature freeze until the next release. |
Limited testing due to battery issues with my laptop. I am not sure how it scales with greater mass, but sprinting felt pretty good in the few generations I played. Great job implementing a feature that makes movement atleast a bit more engaging than pointing your mouse in a direction and pressing W. One thing though: in the original concept, sprinting was attached to the flagella as an ability rather than a default ability, which synergizes with the flagella providing upgrades changing the behavior of sprinting. Was it an intentional decision to have it be available from the get go? |
Oh I wasn't expecting this to be reviewed. Slight heads up that this will probably have to be changed into a new Godot 4 branch. I don't recall there being a particular reason for why the sprinting is enabled by default, iirc it was just an arbitrary decision that's pretty easy to change if needed. However I did intentionally leave out flagella upgrades for the simple reason that I found them too hard to implement and decided to do them down the line when the basic system was done. |
Brief Description of What This PR Does
This PR implements sprinting and cell strain to the game.
To do:
Maybe:
Related Issues
https://forum.revolutionarygamesstudio.com/t/organism-stamina-mechanics/757/7
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.