Skip to content
This repository has been archived by the owner on Jun 7, 2024. It is now read-only.

Add an event editor #41

Closed
wants to merge 51 commits into from
Closed

Add an event editor #41

wants to merge 51 commits into from

Conversation

rossjrw
Copy link
Member

@rossjrw rossjrw commented Apr 24, 2021

  • Create a spec for events and interactions
  • Create a UI for managing and creating interactions
  • Load existing interactions from file
  • Transmit changes to the root events object
  • Add a save button an autosave that backs up the overwritten file
  • Add a UI thingy for each setting with dropdowns with possible values and stuff
  • Perform input validation
  • Make the UI readable, at least a bit - it's pretty terrible atm. Backgrounds, colours and tasteful emoji would help
  • Add collapsibles to hide rarely-needed sections, with a preview of the info contained within in the <summary>
  • Test that every component works and can read/write, totally automatically, using a generated sample event that contains every possible structure - it is essential that I get this right

  • Create a component for each part of the events spec (check when the component is created even if it's not yet complete):
  • Event
  • Interaction
  • Delay
  • Conditional
  • Condition
  • Action
  • Option
  • MessageTimingControl
  • MessageSettings
  • SingleMessage
  • MessageGroup
  • Variables (needs to be added to spec)
  • CounterId
  • FlagId
  • ValueId
  • InteractionId

@rossjrw rossjrw added the feature do something new label Apr 24, 2021
@rossjrw rossjrw mentioned this pull request Apr 24, 2021
27 tasks
@rossjrw
Copy link
Member Author

rossjrw commented May 7, 2021

I'm having some problems with traversing the top-level events list. Because I default to iterating interactions immediately, I totally skip the top level of the events registry, which is the list of top-level events themselves. That's pretty bad, and to be honest, I'm not totally sure how we get from outside the events list into the top level of the events list in the first place.

Jumping directly to interactions is fine, however, I've also skipped the part where multiple event declaration files are read into a single events list. By combining the event declaration files into a fake interactions array inside a fake root event, I will be able to solve both issues at once.

The problem then will be remembering where the events in this list originally came from - after all, if I save this object directly, it will be saved to a single JSON file and the original locations of each event will be destroyed and forgotten. That is to say that this solution would work but it would only work once.

I should find some way of having the separate events files pretend that they're in a single events list from the perspective of the getEventWithIdentifier function.

@rossjrw
Copy link
Member Author

rossjrw commented May 8, 2021

Editing files directly from the browser is not going to work because of sandboxing and shit. I have two options for alternatives:

  1. Make a simple backend and do things that way.
  2. Make a desktop app.

Agh.

I'm going to need to compile to Electron eventually anyway - I guess it's just come up sooner than I thought. There's no point going to the effort of setting up a backend when it's not going to be used again.

The browser cannot actually access these files. A new system (either a
backend or a desktop app) is needed.
@rossjrw rossjrw mentioned this pull request May 9, 2021
2 tasks
There are a lot of changes here and it's all too messy to split into
separate commits. Honestly a lot of it is over my head.

The important bit is that file reading is performed on a separate
process to the renderer, and that that's set up in a secure way. I think
I got it right but that bit's not actually working right now so who
knows. The importanter bit is that I get something that's not just error
messages in the Electron pane: in this case, it's the skeleton of the
Editor component. So, effectively, strip away all the Electron stuff and
this commit just introduces a dirty hotfix to the identifier function.
The contextBridge strips everything but data from objects (including
fs.Dirent objects, including the isDirectory method). This information
needs to be communicated with a primitive value.
It was introducing another layer of complexity that I simply do not need
or want right now.
It's because the new events were being bound after the asynchronous
binders had been called. Moving the binding into a callback and passing
it to the binder was the fix.
Build events editor with Electron
@rossjrw
Copy link
Member Author

rossjrw commented May 12, 2021

I'm now at the point where the data is loaded correctly. The next step is being able to make edits to the data, and then I can implement saving.

Turns out making edits to the data is pretty hard. The way I'm attempting to implement it currently has each component keeping track of an object (e.g. a text field component keeps track of just a string like the event summary; an event editor keeps track of an entire event object). Components in the tree pass the changes they want to make up the tree, eventually reaching the component that keeps track of the event object, which passes a final request to the root object to update the event in its internal store.

Theoretically, this solves the problem of each component needing to know what data it cares about, because each component only needs to handle a single chunk of data and pass any changes to its parent - and data-only components that don't contain any data fields themselves but contain components that do only need to transmit the events and props correctly. That's as opposed to a solution like Vuex, where each component would need to know the exact location of their concerns so they know where to update the data in the store.

The more I'm thinking about it, the more convinced I am that this is the right solution. I think the complexity is actually in my events system, with stuff like createEventAt - there's too many issues there. Instead of using Identifiers for stuff, I should leverage the parent-child system. I should try to use Identifiers only in interactions as way of referring to other interactions.


One problem with this approach is that it relies on a consistent parent-child component relationship existing for everything, however, that's not always the case - events in different files have arbitrary nesting corresponding to their file structure. In these cases, the EventEditor component picks one of the events to edit and records the Identifier pointing to it, which means that I can't totally abolish the Identifier system, because that incredibly crucial part of editing events requires it.

However, that situation isn't exactly necessary. Events don't need to be nested at all - in fact they could be a totally flat structure. Instead of an event looking like ["rootEvent", "", "characters", "breach", "introduction"] or w/e, it could be just "characters/breach/introduction" as a single string. That would avoid the weird double-root empty-string shenanigans too.


I have a few choices for wrangling the root events object:

  1. Just a plain list. Each event would store where it comes from in a location key or something. Downside is that the event objects have to be edited in-memory to contain a location key that they wouldn't contain normally, and that feels messy.
  2. Just a plain list, but it doesn't store any location info at all, and the proxy handler handles it all. Downside is that it'll be difficult to then display where each event is located in the UI.
  3. A dict with path keys.
    1. The keys are the full path to the event, including the filename. Downside is information duplication between the event id and the path - although, that being said, it otherwise wouldn't be possible to enforce that the event id matches the filename, which otherwise might be erroneously assumed.
    2. The keys are the path to the event's dir, and the values are arrays of events, representing each event in that dir. Downside is that this complicates the structure a bit and that's what I'm trying to avoid.

I pick 3.i.


In the future when I've got lots of events, I should come back to 3bc33ac and see what the old event selector UI would've looked like.

@rossjrw
Copy link
Member Author

rossjrw commented May 13, 2021

Noting that both of the sample events, despite having differences between them, appear as identical.

I have done a little investigation and found that:

  • the problem is not in the event prop passed to EventEditor
  • the problem is not in getEvent returning the wrong event
  • the problem could be in the file proxy having bound data for one event to both events

I noticed that, when reading the contents of files transmitted over the
IPC from the file read/write channel in the Electron preload script,
even though I had two different events defined, they were both
identical. I was able to work out that the file contents were likely
being contaminated on account of sharing a data channel. By including
the file path in the data channel name as a unique indentifier, I can
guarantee that the read event for a file should go to the right place.

Curiously, I was already using one-time response events to communicate
with the Electron backend, so I would've expected this bug to result in
events being in the wrong order rather than being duplicated.
@rossjrw rossjrw mentioned this pull request May 14, 2021
This commit gets as far as creating the EditConditional component and
seeing if its core conceit - that is, attaching the wanted component as
a dynamic component - works. It does. But there's still a lot of work to
be done, including creating dynamic components for each term of the
conditional and propagating events back upwards which at the moment is
entirely unimplemented.
@rossjrw
Copy link
Member Author

rossjrw commented May 19, 2021

I've got a pretty serious issue.

There are several nodes in my event tree that can accept a couple of different types. There's also Conditional, which is the only generic type in the tree, which is sort of the same issue.

Through whatever magic, I can easily determine for any node (i.e. parent component) which node the data indicates it contains (i.e. which child component to render). But actually getting the data from the parent into the child is more difficult.

My approach right now is to use a dynamic component and have a method in the parent that determines what kind of component it is. For example, the SingleMessage node represented by EditSingleMessage.vue has a text property that is of type string | Conditional<string> (which are represented by FieldText.vue and EditConditional.vue respectively, and the implementation of that in EditSingleMessage.vue might look like this:

<!-- EditSingleMessage.vue -->
<template>
  <component
    :is="makeComponent().is"
    v-bind="makeComponent().attrs"
  />
</template>

<script>
import FieldText from "./FieldText.vue"
import EditConditional from "./EditConditional.vue"

makeComponent() {
  if (isConditional(this.text)) {
    return {
      is: EditConditional,
      attrs: {
        // whatever attributes EditConditional wants
      }
    }
  }
  return {
    is: FieldText,
    attrs: {
      // whatever attributes FieldText wants
    }
  }
}
</script>

Problem is, 'whatever attributes' is pretty nebulous, and there's really no way to type it. It works, because the Vue template is a black hole that sucks up and destroys all type information. But there's no way of knowing before runtime if the objects I'm creating will work, and even if they do, there's nothing that will tell me when I break them.

I'm really looking forward to making the component tree for the rest of the game, because it's going to be so far divorced from all of these problems.

Another thing is that this functionality is going to come up a lot. Any node that can accept multiple kinds of subnode is going to need it, and that's about half of them, plus more later if I decide I want it. So it's really important that I can get type validation for this.

What should I do?

Ideas, spitting them out as I think of them.

  • Something to do with render functions. What? No idea. Decouple these things from SFCs, maybe. It'd make it more difficult to build these things, although (currently) the structure isn't very complicated.
  • Define the the types I need for stuff in each component, and then import that where I need it. Would mean duplicated type information. Still might have a problem in passing special props like @update:value and stuff.
  • Try another JS framework. I suspect Svelte will have better support for these things. I can still use Vue for the main game, as the point of the project is to use an industry-standard framework and Svelte isn't yet at that level. But it's a bit weird to have a project set up for two frameworks - right?
  • Switch from WebStorm to VSCode. There's a language extension for Vue called Volar that might just magically fix all these issues. However, it's LSP-based, so it won't work with WebStorm.

@rossjrw rossjrw force-pushed the event-editor branch 2 times, most recently from fcaa60a to 1ee5e74 Compare May 21, 2021 12:20
Dynamic components must now declare a separate type for their required
properties (which will likely match their props), and components that
consume them must have a method that outputs a range of components
matching these interfaces.

Note that currently only static downwards properties are implemented; I
haven't implemented update events and don't know if they'll even work.
@rossjrw
Copy link
Member Author

rossjrw commented May 23, 2021

So I made a complete mess of myself making a type system. There was duplication everywhere and most of the types - being effectively-untyped Vue components - were actually useless, so although I was getting presumably-accurate typechecking errors, I've no idea how useful they actually were.

Instead, I've moved as much type information as possible into the base DynamicComponent type and I'm making heavy use of generics. I'm not done yet, but I think this new system is much better. It's certainly a lot easier on the eyes.

@rossjrw
Copy link
Member Author

rossjrw commented May 26, 2021

Alright, the better type system is here, and now it's finally time to work on the update events.

They're unimplemented currently, and the current status is that text fields do change when you edit them, but they reset when changing event.

My test node for update events is going to be the line "We're not currently testing." The tree for this node is EditorEditEventEditInteractionEditMessageGroupEditConditionalFieldText, so I need to make sure that events work between all of those nodes. My understanding is that EditConditional is effectively a pass-through, and its events are not defined there but in each parent component that uses it.

I recall that someone advised me to use computed properties with getters and setters for this sort of thing, and in fact that's what I've already done for FieldText. So I should maybe use that approach where possible, instead of calling an update method. We'll see how things go.

Once update events are done, the best way to immediately test it would be to implement the save system, and then check the diffs. Until then I can't be sure what changes have bubbled to the root and which are stuck in the prop.


Also, another thing I just remembered: this event chain is for updates, but I also need to handle create, destroy and re-order events.

@rossjrw
Copy link
Member Author

rossjrw commented May 30, 2021

Regarding backing up events files.

I switched from the 'save button saves stuff' to an 'always be autosaving' approach, because I realised that's what the Proxy is set up to handle and I don't actually need a Proxy system at all if I'm not doing that.

However, I also had a concern about not making too many backup files, because that could get overwhelming pretty fast. I decided it would be a good idea to round the backups to a timestamp, in this case to the nearest half hour. The files are hashed with that time and the current commit hash, so I'm making one backup file every half hour OR one backup file per commit, whichever comes first.

However, the way I've got it implemented right now means that while I only create one backup file per half hour, that file contains only the most recent change I made. So it's pretty useless for undoing a half-hour's worth of changes.

Instead, I could create a backup with the very first change, and then have any further backups fail until that half hour has elapsed. But that would make the first of such backups pretty much useless, as it'd be effectively identical to the one at the previous commit, and I'd have to wait half an hour before getting a more useful one. Combining this approach with more frequent backups e.g. every 10 minutes should be fine, though. And it won't make extraneous backups if I go ten minutes without making any changes, which is fairly reasonable.

This allows me to use the 'value' keyword to refer to a floating point
value, which is more accurate for both types of variable.
@rossjrw
Copy link
Member Author

rossjrw commented Jun 19, 2021

After a huge rethink and conceptual rework of what I want this editor to be (detailed in my Notion planning document), I'm going to not only extract this editor out of the base pull request (#35) but actually extract it to a separate repository.

Then I'm going to totally change everything about it based on the new requirements, and rebrand it as an entirely separate thing, which the Maitreya project will consume.

It's going to be a tough job extracting the changes out of this PR and into a separate repository. Any changes that affect the main Maitreya project I guess I'll try to append back onto #35.

@rossjrw
Copy link
Member Author

rossjrw commented Jun 20, 2021

I have extracted the event editor out to its own repository: https://github.com/croque-scp/convomap

Other than the event editor, this PR doesn't do anything useful beyond a change to the eslint config (#46) which I'll cherry-pick at some point. Commits have been copied to the new repository. Closing this PR.

@rossjrw rossjrw closed this Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature do something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant