Skip to content

Simplify underwater crouch jump helpers#2261

Merged
osse101 merged 3 commits intovg-json-data:masterfrom
blkerby:underwater-crouch-jump
Aug 26, 2025
Merged

Simplify underwater crouch jump helpers#2261
osse101 merged 3 commits intovg-json-data:masterfrom
blkerby:underwater-crouch-jump

Conversation

@blkerby
Copy link
Copy Markdown
Contributor

@blkerby blkerby commented Aug 25, 2025

The existing helper name h_underwaterCrouchJumpWithFlashSuit seems misleading because it suggests a requirement to have a flash suit, when all it means is that you may have a flash suit. It is defined as {"tech": "canCrouchJump"}, i.e. simply the crouch-jump tech without the {"noFlashSuit": {}} requirement that normally comes along with it. This seems to have led to some confusion where we are using redundant blocks like this:

{"or": [
  "canCrouchJump",
  "h_underwaterCrouchJumpWithFlashSuit"
]},

which expands to

{"or": [
  "canCrouchJump",
  {"tech": "canCrouchJump"}
]},

where it becomes clear that the first case of the or is unnecessary.

This PR proposes that we rename the helper to h_underwaterCrouchJump and replace blocks like this with simple uses of the helper. It also adds a h_underwaterCrouchJumpDownGrab and likewise replaces overly complicated blocks with simple uses of the helper.

This PR only addresses existing uses of the helper h_underwaterCrouchJumpWithFlashSuit. We will need to go through all the crouch jump uses, to classify them as underwater or not, but that can be done in a separate PR; I wanted to make sure first that everything is all right with the proposed helpers.

kjbranch
kjbranch previously approved these changes Aug 25, 2025
Comment thread helpers.json
]
},
{
"name": "h_underwaterMaxHeightSpringBallJumpWithFlashSuit",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a shame to see this one go. It was such a great name. lol

Comment thread helpers.json
"name": "h_underwaterCrouchJumpWithFlashSuit",
"name": "h_underwaterCrouchJump",
"requires": [
{"tech": "canCrouchJump"}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like how it looks like no tech is involved, when we can slot canCarryFlashSuit in here.
The note and purpose are based on flash suit, but the important parts of the helper do not reference flash suits. The value is in the lack of NoFlashSuit which you look somewhere else to see.

So I suggest adding OR:[ NoFlashSuit, canCarryFlashSuit]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The way I look at it is that essentially canCarryFlashSuit is implicitly required on every strat that carries a flash suit, so it would be redundant to add it as a requirement here. Although, since it would be inefficient to check for the canCarryFlashSuit tech on every strat, more specifically the way it's working is that we're checking it at the time the flash suit is gained: every method of gaining a flash suit will come with a canCarryFlashSuit requirement. This means that in any scenario where you could possibly have a flash suit, the canCarryFlashSuit tech will already be satisfied, so we should not need to add it as a requirement anywhere except as a dependency on the tech for gaining a flash suit.

I made an attempt at updating the dev note; does this resolve the problem?

@osse101 osse101 merged commit e47a5b6 into vg-json-data:master Aug 26, 2025
1 check passed
@blkerby blkerby deleted the underwater-crouch-jump branch February 15, 2026 00:06
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.

3 participants