-
Notifications
You must be signed in to change notification settings - Fork 58
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 Subsplits #168
Draft
CryZe
wants to merge
7
commits into
LiveSplit:master
Choose a base branch
from
CryZe:subsplits
base: master
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.
Draft
Implement Subsplits #168
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
CryZe
commented
Apr 25, 2019
/** | ||
* Describes whether the row should be considered an even or an odd row. | ||
* This is useful for visualizing the rows with alternating colors. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This talks about rows, but in horizontal, we don't even use rows.
CryZe
commented
Jun 28, 2019
For now all the subsplit information is discarded to the point where only normal segment names are left.
The individual subsplits sections are now called segment groups that now exist in the individual form of the SegmentGroup and combined form of the SegmentGroups type. The groups are stored in the Run.
Dealing with the index based group ranges is not ergonomic. Therefore this commits adds an iterator that returns all the groups as views of segment slices. Also segments that are not part of any groups become pseudo-groups of single segments. The splits component should be able to build on top of this.
All the splits without any segment groups behave like they did before and the segment groups now properly open up and close as they are being entered. This is done by transforming the segment groups iterator and implementing an iterator on top that flattens it with a specific segment in focus that determines the group that is supposed to be open. After this flattening almost all the original logic remains, but now with the current segment index and the run's length instead being based on this flattened segment group representation.
It seems like we should have them as a separate block in there, such that old parsers will have no problem simply ignoring that new block and parsing the splits regardless. This however has the disadvantage that the groups can now overlap, which we will have to account for when parsing.
Parsing the segment groups works now and it automatically makes sure the ranges are sorted and don't overlap. There are two remaining problems here though. One is that the group name doesn't differentiate between being empty and not existing here. I don't think we want to differentiate those two case long term anyway as it doesn't make much sense in the Run Editor to do so either. The other problem is that it parses segment groups right now that may be out of the total amount of segments' range. That shouldn't cause any major problems, but those ranges probably don't interact well with the Run Editor, so we need to be wary of that.
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.
stalled
The work is not making any progress.
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.
This is a work in progress implementation of Subsplits. Instead of encoding the subsplits information into the segment names we extend the run format properly so the subsplits information can be stored. This is the roadmap for this Pull Request:
Closes #36