-
Notifications
You must be signed in to change notification settings - Fork 45
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
improvement of residual-chart hover info #579
Open
ds26gte
wants to merge
101
commits into
mainmast
Choose a base branch
from
horizon
base: mainmast
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Intended for embedding use cases, the type is: type InitialState = { editorContents: string, interactionsSinceLastRun: string } On load the editor will automatically run definitions with the given text and then run all the given interactions. Currently only enabled in embedding contexts. Need state for two more cases: - Editing definitions but haven't run at all yet - Editing definitions after a run
Co-authored-by: Irv Katz <[email protected]>
One thing that's challenging is the use of on('change') on CodeMirror, which triggers for calls to setValue(). In the events API, we change these to all use replaceRange with an explicit `origin`. However, there are setValue() calls elsewhere in the codebase. We could ignore these by checking the origin of "setValue", but this could be a mistake if we do want to propagate around updates like those. Need to think about this. For now, filter out ones that come in before we are initialized, and wait for the reset event to start sending things out. This makes sure that the initialization of setting e.g. `use context current` doesn't get sent out as an event. Sometimes we get "empty" resets from VMT, and this commit just filters those out. But it's not clear when exactly that happens.
- Don't propagate setvalue events; these are coming from page setup, but we want to wait for our 'reset' - Don't setValue at all if the page is “controlled”. Add a new #-parameter #controlled=true that tells the page to not do its usual initialization and instead wait for the reset events - The first 'reset' has a state of '', so deal with that appropriately to handle initialization in events.js
This is a total hack, and should probably be disabled. However, for demo servers it can be incredibly valuable to have share links from e.g. code.pyret.org work. Currently they won't because the Google credentials are wrong. But, we can simply configure these fetches to ask for the corresponding resource on CPO. Consider trying that, then falling back to looking on this server if needed?
(definitionsAtLastRun = false means the program has not been run. We can do one better by setting it to starter2024 and running it)
- Hide and show run/stop on lose/gain control - Make editor and REPL non-editable when losing control - Add event listeners on editor and repl to prompt for control when not in control - Fix a bug where the old REPL contents could be included in the state when it should have been empty just after an interaction
- Need to make sure we report them as not present if window.MESSAGES isn't - Lots more we can do with modified time, etc, this is just copying from gdrive stuff
The high-level goal here is a clean solution to having an in-control user move everyone along if a reactor or chart modal is open. The idea is to trigger a stop if a user is (a) not in control and (b) a run or runInteraction event comes in. This means that all users can interact with the chart, reactor, etc on their own, and won't be interrupted if the in-control user stops theirs. But as soon as the in- control user moves on to run another interaction or use the Run button, it will forcibly stop the other clients. This is *not* perfect. Other designs might work better. The best case for this design is that people get told in chat that they should close on their own, and the forced stop is just a way to sync back up. Some weaknesses and tradeoffs discussed below: 1. For a main program like this, if the forced stop happens the `r` variable ends up not being defined, which isn't great. r = reactor: init: 1, on-tick: _ + 1 end r.interact() 2. We could capture the “close” event of the modals and try to propagate it around. This is tricky because: - It's hard to account for all the ways the modal can close (click, `stop-when` trigger, reactor ending in error, etc) - The in-control user's game might end early (because of stop-when), and it could be nice to let others continue. 3. We could trigger something less drastic than a full “stop” event. For example, we could just do JSworld.shutdown({ clean: true }) when the next interaction happens. However, a single program can have multiple reactors that open in serial, conditional on what actions happened. This happens in practice! For example the car simulator in physics starts several reactors for several different initial conditions. And in general it's nice to have a solution that for sure gives control back to the REPL. 4. Another option would be to: - Queue the interactions that come in while the reactor is running, and run them once the reactor stops. - Only do a force stop when the *Run* button is clicked. This would make it so folks can independently run their animations, and when they quit the next interactions would start to process. If the in control user instead re-runs the whole program, *then* all others get stopped. The queue is a bit nontrivial to maintain, but this may be a better user experience for when a "re-sync" ought to happen. The queue is also nice architecturally because there are potential issues of things taking different times to run on different machines for long-running programs. This could be a potential issue for e.g. loading sheets lagging on some folks' computers and causing them to get stopped unnecessarily, and the “queue until rerun” would help a lot with that.
tweak CSS for even-numbered rows to make them more apparent (#185)
set the title attribute of the filename element to be the FULL filename
- Add explicit state that tracks if we are in control or not, and don't disable editors if we are in control - After setting interactions, if we're in control, focus it.
…ch is the right re-simulation of starting from scratch
I think this is the issue, I'm getting this when using the published VScode web extension and embedding CPO. "not-set cross-origin-resource-policy: Because your site has the Cross-Origin Embedder Policy (COEP) enabled, each resource must specify a suitable Cross-Origin Resource Policy (CORP). This behavior prevents a document from loading cross-origin resources which don’t explicitly grant permission to be loaded." The COEP is enabled by github.dev (I think), so this says “yup please embed us”
"not-set cross-origin-embedder-policy: To embed this frame in your document, the response needs to enable the cross-origin embedder policy by specifying the following response header: Cross-Origin-Embedder-Policy: require corp "
If we have a sendRpc target (like VScode) we can try to fill in the fs library to some extent. We won't be able to get full coverage because we can't do synchronous RPCs (so e.g. filelib won't work transparently) But we can do a lot of it, and we can probably update the pyret-lang files to use a reasonable subset that we can polyfill with RPCs to embedding contexts that do have a filesystem
- Add a `reportAnswer` field to `runInteraction` that requests the interaction report its result back. For now, this is the innerText of the last element of the #output, which is probably enough for a lot of things, but a little kludgy. - For the “recovery”code that runs when state gets desynced, ONLY run it when we are not in control. For embedded instances that are allowing editing, it's better to just trust our current state rather than expecting embedders to keep all state and message sequencing up to date.
- Relies on readFile from the embedding context that gives back the Uint8Array, rather than the UTF8-stringified result of the file - Adds image-file, which gets this Uint8 buffer from a path and converts it to base64 - Uses a (small for now) hardcoded list of extensions to trust. This is what browsers do in a file upload dialog (see https://developer.mozilla.org/en-US/docs/Web/API/Blob/type, “Note: Based on the current implementation, browsers won't actually read the bytestream of a file to determine its media type. It is assumed based on the file extension; a PNG image file renamed to .txt would give "text/plain" and not "image/png". Moreover, blob.type is generally reliable only for common file types like images, HTML documents, audio and video.”)
Pass an empty opts to indicate that raw Uint8 data should come back, rather than having a separate endpoint.
csv-parser library updates
image-file
Residual hover info
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.