Skip to content
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

Implement Custom Variables #201

Merged
merged 3 commits into from
Jan 7, 2020
Merged

Conversation

CryZe
Copy link
Collaborator

@CryZe CryZe commented May 17, 2019

This Pull Request adds custom key value pairs to Runs. They can be created by the runner in the Run Editor and can store arbitrary string based values that the runner wishes to store with the splits. Components can then use these variables to visualize them. In particular, the text component can visualize these variables as key value pairs. Variables can also be provided by auto splitters, components or through other means. The automatic variables are temporary in nature and are therefore never stored in the splits files. The user also can not edit them. There also is an orthogonal scope per variable that binds the variable's value only to single attempts and resets its value between each one. This can be useful for per attempt counters.

Closes #200

  • Rough prototype
  • Settings for text component
  • Saving custom variables
  • Loading custom variables
  • Adjustments based on proposal
  • Run Editor UI in LiveSplit One
  • Documentation
  • Tests

@CryZe CryZe added enhancement An improvement for livesplit-core. feature A new user visible feature for livesplit-core. labels May 17, 2019
@CryZe
Copy link
Collaborator Author

CryZe commented Jun 27, 2019

Proposal

After experimenting with the implementation of the Custom Variables, here's a final proposal of how they possibly should work. I'll adjust the final implementation based on what we decide here.

Variable Kind

There are two kind of variables: Temporary or Permanent.
A permanent variable is always stored in the splits file and is never removed unless the user explicitly removes it via the Run Editor. If a transition to a different kind of variable is perceived, only certain transitions are allowed: A permanent variable can never become a temporary one, while a temporary variable can become a permanent one. If a variable is manually created in the Run Editor, it is always considered permanent. If the variable is created via a script, it is considered temporary by default, unless otherwise stated. The Run Editor only shows permanent variables. Creating a variable in the Run Editor that is already a temporary variable keeps its value.

Variable Scope

A variable's value has one of the following scopes: Always or Per Attempt. These can be extended to other values in the future, such as Per Segment or Per Runner but the MVP is only implementing these. The scope defines when a variable has a value. Variables that are scoped to a single attempt can only have a value during an attempt and the value automatically gets removed once the attempt is over. The value of an attempt scoped variable is not stored in the splits file and is not shown in the Run Editor.

@CryZe CryZe requested a review from wooferzfg June 27, 2019 14:00
@CryZe
Copy link
Collaborator Author

CryZe commented Jun 27, 2019

@wooferzfg I'd like you to read through the proposal and want your opinion on it.

@CryZe
Copy link
Collaborator Author

CryZe commented Jun 27, 2019

I almost feel like we should have design documents for every major design decision we do. And then have a test per sentence, that actually references the sentence of the design document. Not sure how those would interact with module and doc comments then though. I guess I'll see if I can fit the proposal fully into doc comments and module documents. If that works out, then there might not be a need for separate design documents.

@wooferzfg
Copy link
Member

  • What's the functional difference between Temporary and Permanent variables? Is it just that Temporary variables don't show up in the Run Editor?
  • Do we ever have a case where a Permanent variable is scoped Per Attempt? I can see where a Permanent variable would be scoped Per Segment (to save some sort of custom info), but Per Attempt seems inherently temporary to me. Also, "The value of an attempt scoped variable is not stored in the splits file and is not shown in the Run Editor" sounds like it's describing a Temporary variable.
  • Even though we have an Always scope, I'm assuming that variables are saved per splits file. If so, we should consider the expected behavior when switching between splits files. What should components show if the variable it was displaying no longer exists?

@CryZe
Copy link
Collaborator Author

CryZe commented Jun 30, 2019

What's the functional difference between Temporary and Permanent variables? Is it just that Temporary variables don't show up in the Run Editor?

Temporary variables are usually provided by auto splitters and other scripts. So they are mostly statistical in nature and don't need to be saved. This also means you don't usually want to edit them or create them yourself.

Do we ever have a case where a Permanent variable is scoped Per Attempt?

Mmh, I'd say that you create a death counter variable in the run editor, make it per attempt scoped and then it resets between attempts. However now that I'm thinking about it, we may not actually need to create it in the run editor. Just the layout settings or whatever might be sufficient.

However more permanent counters would be created via the run editor, so maybe it is worthwhile having the notion of a permanent variable with a temporary value regardless, which is the whole idea behind the separation of the two terms here. (The terms can change if you come up with better names)

What should components show if the variable it was displaying no longer exists?

The text component for example simply shows a dash, just like other key value pair components (info text components) do. But yeah components that use variables need to be able to handle them not existing / having a value.

@wooferzfg
Copy link
Member

wooferzfg commented Jun 30, 2019

Per Attempt and Per Segment kinda seem like different types of scope to me. Per Attempt describes a one piece of information that gets cleared when out of scope, while Per Segment describes multiple pieces of information that are limited to each segment but don't actually get cleared (at least in the case of Permanent variables?).

@CryZe
Copy link
Collaborator Author

CryZe commented Jun 30, 2019

No, they would get cleared the same way, but they could be stored in the segment history before being cleared. But every other variable could as well. So I don't think there's a fundamental difference there. Unless I'm misunderstanding what you are saying.

@CryZe
Copy link
Collaborator Author

CryZe commented Jun 30, 2019

Maybe we should keep the notion of the scope out of this PR as this is really about the automation of when the variable should be cleared. But maybe that's something that individual scripts / components should decide themselves. Like setting the death counter to per attempt / per segment or whatever in the counter settings (not sure if it will be a component or whatever yet, but no matter what it will be, it will have settings)

@CryZe CryZe force-pushed the custom-variables branch from ca3ee81 to a1c064a Compare July 28, 2019 14:08
@CryZe
Copy link
Collaborator Author

CryZe commented Oct 25, 2019

@wooferzfg Considering this is mostly just blocked on the Run Editor changes in the web LiveSplit One, and you seem to actively make changes to that codebase, I'd appreciate if you'd pursue this next, if you want to. I need to check what the state of this is on my side (especially since I already had half of the changes in LSO, so you can probably just pick up that branch).

@wooferzfg
Copy link
Member

Yeah, I can definitely work on this. We should talk about the UI design for these changes though.

@CryZe
Copy link
Collaborator Author

CryZe commented Dec 18, 2019

@wooferzfg It probably makes sense to push to the branch in my PR instead of on the branch in this repo. (As you can see none of your changes show up as part of this PR).

Also in general when you are working on your own PRs, I'd rather prefer if you worked in a fork. The problem is when people fork the repo, they get a full snapshot of all the branches in it, which means they have a bunch of in-progress branches in their fork, that forever stay in-progress like that. I'd like to just have the master branch here (though dependabot sometimes creates a few branches here too, which I also don't like).

@CryZe
Copy link
Collaborator Author

CryZe commented Jan 6, 2020

Not sure why CI is not running anymore?! Maybe it'll start after rebasing again.

Well they work again 🤷‍♂️

CryZe added 2 commits January 7, 2020 00:27
This Pull Request adds custom key value pairs to Runs. They can be
created by the runner in the Run Editor and can store arbitrary string
based values that the runner wishes to store with the splits. Components
can then use these variables to visualize them. In particular, the text
component can visualize these variables as key value pairs. Variables
can also be provided by auto splitters, components or through other
means. There is a distinction between permanent and temporary variables.
Most automatically provided variables are temporary by default, but they
can be turned into permanent ones. The permanent ones can be created by
the runner in the run editor and will be stored in the splits files
unlike the temporary ones.
@CryZe CryZe force-pushed the custom-variables branch from 1edcf4a to d313897 Compare January 6, 2020 23:28
@CryZe CryZe marked this pull request as ready for review January 6, 2020 23:30
This temporarily mitigates the floating point inconsistencies introduced
by switching to lyon 0.15.

cc LiveSplit#287
@CryZe CryZe merged commit 7b0ca7a into LiveSplit:master Jan 7, 2020
@CryZe CryZe added this to the v0.12 milestone Jan 7, 2020
CryZe added a commit that referenced this pull request Nov 14, 2021
- Runs now support custom variables that are key value pairs that either the
  user can specify in the run editor or are provided by a script like an auto
  splitter. [#201](#201)
- There is now an option in the run editor to generate a comparison based on a
  user specified goal time. This uses the same algorithm as the `Balanced PB`
  comparison but with the time specified instead of the personal best.
  [#209](#209)
- Images internally are now stored as is without being reencoded as Base64 which
  was done before in order to make it easier for the web LiveSplit One to
  display them. [#227](#227)
- The Splits.io API is now available under the optional `networking` feature.
  [#236](#236)
- All key value based components share the same component state type now.
  [#257](#257)
- The crate now properly supports `wasm-bindgen` and `WASI`.
  [#263](#263)
- There is now a dedicated component for displaying the comparison's segment
  time. [#264](#264)
- Compiling the crate without `std` is now supported. Most features are not
  supported at this time though.
  [#270](#270)
- [`Splitterino`](https://github.com/prefixaut/splitterino) splits can now be
  parsed. [#276](#276)
- The `Timer` component can now show a segment timer instead.
  [#288](#288)
- Gamepads are now supported on the web.
  [#310](#310)
- The underlying "skill curve" that the `Balanced PB` samples is now exposed in
  the API. [#330](#330)
- The layout states can now be updated, which means almost all of the
  allocations can be reused from the previous frame. This is a lot faster.
  [#334](#334)
- In order to calculate a layout state, the timer now provides a snapshot
  mechanism that ensures that the layout state gets calculated at a fixed point
  in time. [#339](#339)
- Text shaping is now done via `rustybuzz` which is a port of `harfbuzz`.
  [#378](#378)
- Custom fonts are now supported.
  [#385](#385)
- The renderer is not based on meshes anymore that are suitable for rendering
  with a 3D graphics API. Instead the renderer is now based on paths, which are
  suitable for rendering with a 2D graphics API such as Direct2D, Skia, HTML
  Canvas, and many more. The software renderer is now based on `tiny-skia` which
  is so fast that it actually outperforms any other rendering and is the
  recommended way to render.
  [#408](#408)
- Remove support for parsing `worstrun` splits. `worstrun` doesn't support
  splits anymore, so `livesplit-core` doesn't need to keep its parsing support.
  [#411](#411)
- Remove support for parsing `Llanfair 2` splits. `Llanfair 2` was never
  publicly available and is now deleted entirely.
  [#420](#420)
- Hotkeys are now supported on macOS.
  [#422](#422)
- The renderer is now based on two layers. A bottom layer that rarely needs to
  be rerendered and the top layer that needs to be rerendered on every frame.
  Additionally the renderer is now a scene manager which manages a scene that an
  actual rendering backend can then render out.
  [#430](#430)
- The hotkeys are now based on the [UI Events KeyboardEvent code
  Values](https://www.w3.org/TR/uievents-code/) web standard.
  [#440](#440)
- Timing is now based on `CLOCK_BOOTTIME` on Linux and `CLOCK_MONOTONIC` on
  macOS and iOS. This ensures that all platforms keep tracking time while the
  operating system is in a suspended state.
  [#445](#445)
- Segment time columns are now formatted as segment times.
  [#448](#448)
- Hotkeys can now be resolved to the US keyboard layout.
  [#452](#452)
- They hotkeys are now based on `keydown` instead of `keypress` in the web.
  `keydown` handles all keys whereas `keypress` only handles visual keys and is
  also deprecated. [#455](#455)
- Hotkeys can now be resolved to the user's keyboard layout on both Windows and
  macOS. [#459](#459) and
  [#460](#460)
- The `time` crate is now used instead of `chrono` for keeping track of time.
  [#462](#462)
- The scene manager now caches a lot more information. This improves the
  performance a lot as it does not need to reshape the text on every frame
  anymore, which is a very expensive operation.
  [#466](#466) and
  [#467](#467)
- The hotkeys on Linux are now based on `evdev`, which means Wayland is now
  supported. Additionally the hotkeys are not consuming the key press anymore.
  [#474](#474)
- When holding down a key, the hotkey doesn't repeat anymore on Linux, macOS and
  WebAssembly. The problem still occurs on Windows at this time.
  [#475](#475) and
  [#476](#476)
@CryZe CryZe deleted the custom-variables branch November 16, 2021 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement for livesplit-core. feature A new user visible feature for livesplit-core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement a Custom Variable System
2 participants