Skip to content

add basic support for an intuitive JSON based route format#19

Draft
afontenot wants to merge 10 commits intorhelmot:mainfrom
afontenot:json-routes
Draft

add basic support for an intuitive JSON based route format#19
afontenot wants to merge 10 commits intorhelmot:mainfrom
afontenot:json-routes

Conversation

@afontenot
Copy link
Contributor

I mentioned the possibility of adding JSON support to CelesteMagicTimer in a comment on a previous PR. This adds proof of concept support to celeste_timer.py by accepting a JSON route in the new format and automatically generating a route that works with the existing code.

I'm curious if you have any thoughts about the JSON format I came up with. It seems extremely intuitive to me:

  1. Splits are listed in the file in the order that they actually appear in the livesplitter.

  2. The "level" concept is a built-in natural result of using JSON, instead of using a list with a "level" value. Splits can be nested arbitrarily deep.

  3. All the splits are given in the JSON itself, rather than (awkwardly, IMO) combining the last subsplit of each split with the split itself. No more split.names = ["City", "Chasm"] kind of stuff.

  4. Because it's just text, it's a much easier format for most users to edit to meet their own needs. It would also be easier for someone to write a route editor designed around the format even if they weren't familiar with the internals of this program.

Again, though, this is just a proof of concept. I think if it seems nice to you and works well it might be worthwhile to write a version of celeste_timer.py that makes it the default, and adds support for text based PB files and so on. I think the natural hierarchical syntax might allow for a cleaner timer implementation. At least, a version designed around using this JSON format would not have to do the rather ugly recursive conversion that my POC does.

Might also be worth coding something that can avoid using eval for common triggers (chapter endings, room entrance, etc etc).

@afontenot
Copy link
Contributor Author

afontenot commented Jul 13, 2021

I added a timer built specifically to use the JSON route format. It seems pretty solid although I'm not happy with every aspect of it. I think it might be reasonable to merge something like this, so let me know what you think if you have a look at it. I tried to comment the timer very extensively to make it easy to maintain for someone who's not me (and therefore not familiar with the code).

It does everything I need, but I've left a couple of FIXMEs in for possible improvements.

Let me know if you like this and I can make any final changes or squash the commits together if you want.

Edit: forgot to mention: this PR also adds support for text based PB files, which is super useful for referencing or editing, as that's sometimes needed.

@rhelmot
Copy link
Owner

rhelmot commented Sep 17, 2021

I'm super unsure how to feel about this one. I think the idea is good, but I feel real uncomfortable inheriting all this code which makes very different design decisions than the ones I made. I see you've still got it marked as a draft, so I'll let you keep working on it while I work on making the pickle-based routes more intuitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants