Fix knot tracking - #159
Conversation
Fix knot tracking for tunnels and threads Knots, tunnels and threads should all have entry tracked, otherwise they don't get per-knot tags. * Set track_not_visit for all frames other than functions. * Added new test TagsAndBranching which tests that tags end up visible in inkcpp for all knot types.
Track non-functions on jump, don't update counts on return. Also: * Remove parameter from visit() which makes it do nothing, just don't call it. * Add named Booleans for everywhere we call jump - 3 parameters of the same type with default values is easy to get wrong otherwise. In future we should change this to a set of flags, but I want to see if the logic can be cleaned up a bit first. It's fiddly.
Can't just run ClangFormat in VS as that makes further changes unrelated to the code changes.
willvale
commented
Jul 3, 2026
OK, this is now in sync with what I have locally. Will keep looking. |
…rrent knot The knot-recording change for tags means that this isn't always true. And if the destination is behind, the scanning code eats the entire story.
willvale
commented
Jul 3, 2026
@JBenda I would have a look at this in detail - it's possible that by tracking the current knot across call/return, I might be breaking assumptions in inkcpp about what the current knot can be. The workaround I added above seems to be fine for inkcpp-test and ink-proof, but I'm not sure if it's missing something in migration because I don't know too much about that. See what you think. |
Ink Proof ResultsThese results are obtained by running the Ink-Proof Testing Suite on the compiled binaries in this pull request.
|
JBenda
commented
Jul 4, 2026
First, thanks for the PR. I took a first look, and it looks good. I should document assumptions more, but to be real, I forgot about knots and functions when writing the migration logic. I will sleep a night and look at the code again, but I will probably merge it as it is. The local variable init for migration is not the finest piece of code, but it should work good enough for the start. PS: Nice spot that we can move the |
willvale
commented
Jul 4, 2026
NB: The reason I wrote this is I'm starting to do some more complicated things with inkcpp in my game project to support dropping and resuming dialogue. So I can interrupt a conversation in the middle, play a "where did you go" line on a secondary runner and then eat dialogue until I reach a "resume" tag or leave a resumable knot. And then continue normally. This relies on line tags (which were working) and knot tags (which didn't work for tunnels, which I use a lot). I still need to do the same kind of thing for resuming backwards (e.g. if there's dialogue that's important, we need to go back to where it started) so it's possible that will generate more PRs. |
Uh oh!
There was an error while loading. Please reload this page.
Re-opening so I can test the workflow, but probably not ready yet.
Locally, everything passes (ink-proof, inkcpp-test) except an obscure failure in one of the migration tests while initialising globals (before the migration or anything interesting happens) which I haven't figured out yet.