numberscope / frontscope

Numberscope's front end and user interface: responsible for specifying sequences and defining and displaying visualizers
MIT License
7 stars 15 forks source link

URL out-of-sync bug in parameter handling in ui2 #403

Closed katestange closed 2 months ago

katestange commented 4 months ago

There's an important bug that I'm having to handle in enhancing turtle, but should properly be fixed in ui2, not in turtle. Here's the situation:

Suppose that inside checkParameters() a parameter should be modified to conform. For example, some visualizers will stop you from putting a "number of terms" that exceeds this.seq.last - this.seq.first (quite reasonably). It would be nice to be able to simply replace a large number entered by the user with the maximum allowed by the sequence, and display this updated value in the parameter pane right away. Note that no default value here is simultaneously reasonable and safe, as many OEIS sequences have very few terms. So if you don't want the visualizer to load up in a default invalid state on such sequences, you want to just update the number of terms (including the default) if it is too large immediately to the sequence's allowable max. This is also a favour to the user, who can't see the max in the UI at the moment. So this is something we probably want to do, and I find myself wanting to do in Turtle.

Now, this check can't be performed earlier than checkParameters() since it needs access to this.seq. So it happens inside checkParameters(), and then you have to call refreshParams() to display the modified value to the user, presumably at the start of presketch(). This works.

Except that in this scenario the URL doesn't update with the correction in refreshParams() and instead shows whatever too-large value the user typed (it appears as the user is typing). So then the URL is out of sync with the visualizer's actual internal state, and hard reloading or sharing the URL will cause a crash (or whatever the bad value will cause) because a reload or fresh load doesn't seem to re-run checkParameters().

gwhitney commented 3 months ago

The description is totally understandable, and I think Chaos is already doing something similar with replacing its first and last indices with those of the sequence when they are out of bounds. Can the bug be exercised with existing Chaos visualizer? Generally speaking, I am happy to work on this, I just need an explicit "recipe" for making the bug occur.

gwhitney commented 3 months ago

Ah, I see, it happens with A371958 under the current Chaos visualizer. That sequence only has 15 terms, so the "last" parameter shows 15. If you type 0 after that in the input field, the URL changes to include last=150 even though the parameter box still shows 15. OK, I will take a look.

gwhitney commented 2 months ago

@katestange would you be able to check if this has now been resolved in ui2, or if you can still reproduce it in this branch? If it seems resolved, feel free to close this issue; if not, please add any further details needed here, or at least a verification that it still exists. Marking this for alpha as it seems it is/was a bug in the new code.

katestange commented 2 months ago

This appears to be resolved in ui2, so I'm going to close the issue.