Skip to content

Make escaper patterns increment the op count - #1172

Merged
IridescentVoid merged 5 commits into
mainfrom
escaper-opcount
Jul 30, 2026
Merged

Make escaper patterns increment the op count#1172
IridescentVoid merged 5 commits into
mainfrom
escaper-opcount

Conversation

@Robotgiggle

Copy link
Copy Markdown
Member

When I turned the escaper patterns into Actions in #1047 I forgot that Action doesn't automatically handle incrementing the op count. This PR fixes that by manually incrementing it, just like in all the other patterns that directly extend Action.

@Robotgiggle Robotgiggle added 1.20 optimization Improvement to existing features or performance (no new features or design work) and removed optimization Improvement to existing features or performance (no new features or design work) labels Jul 27, 2026

@IridescentVoid IridescentVoid left a comment

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.

Code changes seem to work as intended, but I'd like to discuss the balance implications before merging. As it currently stands, these changes significantly disadvantage using introspection and retrospection when nesting fragments of hexes.

@Robotgiggle

Robotgiggle commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Part of the problem was that I made intro and retro increment the opcount even when inside a parenthesized list, which makes very little sense in retrospect. It not only balloons the op cost when deeply nesting but is also just plain unintuitive - normal patterns don't affect the opcount when parenthesized, after all.

I've removed that feature, so now intro and retro only consume ops when actually evaluated on the outer layer. Consideration still uses an op while parenthesized because the entire point of consideration is to have actual behavior when parenthesized instead of just going into the list.

@IridescentVoid IridescentVoid left a comment

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.

Looks good to me.

@IridescentVoid
IridescentVoid added this pull request to the merge queue Jul 30, 2026
Merged via the queue into main with commit c97a251 Jul 30, 2026
9 checks passed
@github-project-automation github-project-automation Bot moved this from 📋 Backlog to ✅ Done in Hex Casting Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants