This PR updates the behavior for songs with drumroll branching
conditions (`#BRANCHSTART r`)
The changes include:
- Correctly setting the `branchInfo` bytes for branching conditions that
occur _after_ the first condition.
- Correctly setting the `branch_points` bytes in the header when there
are only drumroll conditions.
- Correctly setting the `branch_ratio` bytes in the header when total
notes differ between branches.
- Correctly handling #SECTION commands for a number of different corner
cases:
1. #SECTION occurs on its own without a #BRANCHSTART
2. #BRANCHSTART occurs with a #SECTION command
3. #BRANCHSTART occurs without a #SECTION command (and is first branch
condition)
4. #BRANCHSTART occurs without a #SECTION command (and is NOT first
branch condition)
Note: I've added `shoto9` to the test suite, but the TJA file is
structured in a way that the number of measures doesn't match the number
of fumen measures. The TJA needs to be reworked, but that's okay,
because its purpose was just to check that the branching bytes were
correct.
Fixes#40.
Previously, I had to reverse engineer many of the bytes in a valid fumen
header. This meant that I was often working with literal byte values, as
well as guessing at what things meant, calling them "padding" or
"unknown".
However, thanks to #44, I now have a TON of knowledge about the correct
purpose of each byte in the header. This lets me properly document each
value's purpose, and it also lets me parse 4-byte groups as ints, rather
than dealing with individual raw bytes.
In summary, the changes in this PR are:
- Create a new `FumenHeader` class with individual (correctly-named!)
attributes for each 4-byte integer.
- Parse and write the fumen header as one big 520-byte chunk, instead of
handling the header in smaller increments.
- Remove a bunch of dead lines of code that were hardcoding specific
byte values, and replace them with proper integer values.
- Vastly simplify the test for valid headers by removing checks against
individual bytes..
Necessary for #40.
Fixes#44.
This PR adds `genpe.tja` to the test suite, a song that currently fails
due to how `tja2fumen` handles `#SECTION` commands.
We fix the problems with branching commands in
8ad2102f0a. (Making sure that any #SECTION
commands that occur before the first #BRANCHSTART are properly handled.)
This PR also contains a few refactoring commits to improve the clarity
of how the branching logic works, to make further debugging of branching
songs easier.
Fixes#33.
This PR adds a new chart to the test suite (`imcanz.tja`) that uses
`p,0,0`, `p,999,999` _and_ `#SECTION` commands:
- `p,0,0`: Forces the chart into the Master branch (since it's
impossible to fail a 0% accuracy requirement)
- `p,999,999`: Forces the chart into the Normal branch (since it's
impossible to pass a 999% accuracy requirement)
- `#SECTION`: Resets accuracy values for notes and drumrolls on the next
measure. (In practice, this just means that the branch condition is
repeated on the next measure, at least according to the official fumen I
have.)
Note: Only the Oni and Hard difficulties have actually been added to the
test suite. The Normal and Easy charts were too broken to easily match
the official fumens. They will need a lot of work to fix charting
errors, so I'm leaving them commented out for now.
Fixes#27.
Before:
- If there was a #BPMCHANGE, tana's adjustment would be applied to _the current measure's duration_. This would result in very strange looking, impossible durations (e.g. negative)
- Additionally, this method required us to look ahead to the _next_ measure to determine the adjustment.
- Finally, it required us to keep track of two different durations: measureDurationBase (unadjusted) and measureDuration (adjusted)
After:
- Instead, we now leave the measureDuration as purely as "unadjusted".
- Then, we apply the "BPMCHANGE adjustment" _only_ when computing the fumenOffset start of the next measure.
- This requires us to keep track of both the start *and* end fumenOffset (where end = start + duration)
- However, this greatly simplifies and clarifies the code, since we:
- No longer need to "look ahead" to the next measure to compute the offset adjustment.
- No longer need to keep track of two different measureDurations (adjusted/unadjusted)
- Only need to work with a single pair of measures at a time (measureFumen, measureFumenPrev)
- The measure duration (and fumenOffsetEnd) times are now more comprehensible, since any negative offsets are only applied to the fumenOffsetStart value of the next measure.
After:
- Before: 'OFFSET' = 2nd measure fumenOffset
- After: ('OFFSET'-Full measure duration) = 1st measure fumenOffset
Both methods will produce the same results if no #MEASURE/#BPMCHANGE
commands are present in the first two measures. However, if these
commands ARE present, then you will get wildly different results.
This change doesn't affect 4/6 tests, but for 2/6 tests there were
#MEASURE/#BPMCHANGE commands. For these songs, I had erroneously "fixed"
the OFFSET values, when they were correct all along. So, I reverted them
back to their original values.
Fixes#25.
This PR adds support for multiplayer charts, and adds a new multiplayer
TJA/fumen combo for testing.
This PR is quite small, because the only work that needed to be done is
parsing the course metadata. Once the TJA lines are split into courses,
the actual course-by-course parsing logic is identical to the songs that
came before.
Fixes#6.
This PR adds the remaining changes needed to support branching TJA
songs.
- Properly handling `r` (drumroll) based branching conditions
- Handling commands that come after a measure end but _before_ the first
`#BRANCHSTART` command. (This was not previously covered by
`hol6po.tja`.)
As well, it adds a bit of cleanup/refactoring to the changes from the
previous branch PR (#20).
To test these changes, another new branching TJA-fumen pair is added to
the test suite.
Before, I assumed that only mid-measure BPMCHANGE commands would split the measure in two. However, mid-measure SCROLL/GOGO commands *also* split the measure in two.
For non-branching songs, `-1` == `idx_m-1`. But, for branching songs, we will repeatedly visit the same measures to populate the different branches. This means that the relative index `-1` is no longer valid.
This bug wasn't caught because `measureDurationPrev` generally equals `measureDuration` for the first two measures.
However, if there is a BPM change within the first 3 measures, then the wrong `measureDuration` will be used. So, we need to explicitly specify `measureDurationPrev`
This reverts commit af91e9fe47.
I was mistaken. Importing from src is _not_ necessary to perform PyCharm coverage tests. Instead, the package must be installed in editable mode.
The official fumen had some issues where a note ended up in the wrong measure:
- Expected: pos 0.0 on the next measure
- Actual: pos 645 on the prev measure (impossible pos)
They're the same pos, but only pos 0.0 is possible to express in TJA charts, so the fumens themselves had to be updated.
Also, the TJA chart had errors, too:
- The BPMs/SCROLLs in the BPMCHANGE section were incorrect
- Some drumrolls were too short
Fixing these errors caused the failing test to pass.
Instead, we should only update the values once we encounter more notes.
This prevents an issue where a command is used before a BPMCHANGE event:
# 33
# #GOGOEND
# #BPMCHANGE 107
# 33,
If GOGOEND is applied right away, then the first '33' notes will be affected. But, we only want the last '33' notes to be affected.
This will _actually_ perform an in-place test run, rather than importing from the installed package. This is necessary for PyCharm's coverage to ework correctly.