Downgrading to msdparser==2.0.0b1 seems to resolve the issue for me.

Python 3.9.1 (tags/v3.9.1:1e5d33e, Dec  7 2020, 17:08:21) [MSC v.1927 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import simfile
>>> simfile.open("Xuxa.sm")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\simfile\__init__.py", line 89, in open
    return open_with_detected_encoding(
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\simfile\__init__.py", line 137, in open_with_detected_encoding
    return (load(file, strict=strict), encoding)
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\simfile\__init__.py", line 63, in load
    is_ssc = _detect_ssc(peek_file)
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\simfile\__init__.py", line 46, in _detect_ssc
    for first_key, _ in parser:
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\msdparser\__init__.py", line 202, in parse_msd
    for token, value in lex_msd(
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\msdparser\__init__.py", line 366, in lex_msd
    chunk = textio.read(4096)
  File "C:\Users\w\AppData\Local\Programs\Python\Python39\lib\site-packages\simfile\_private\tee_file.py", line 13, in __getattr__
    return getattr(self._file, name)
AttributeError: 'itertools._tee' object has no attribute 'read'

First off, thanks for building this library! It's been remarkably easy to use for simfile introspection.

I ran into this unfortunate edge case while parsing through DDR A20 PLUS content:

Traceback (most recent call last):
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/simfile/__init__.py", line 89, in open
    return open_with_detected_encoding(
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/simfile/__init__.py", line 137, in open_with_detected_encoding
    return (load(file, strict=strict), encoding)
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/simfile/__init__.py", line 67, in load
    return SMSimfile(file=file, strict=strict)
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/simfile/base.py", line 139, in __init__
    self._parse(parse_msd(
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/simfile/sm.py", line 159, in _parse
    for (key, value) in parser:
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/msdparser/__init__.py", line 117, in parse_msd
    ps.write(char)
  File "/Users/vyhd/.pyenv/versions/3.9.2/lib/python3.9/site-packages/msdparser/__init__.py", line 62, in write
    raise MSDParserError(f"stray {repr(text)} encountered")
msdparser.MSDParserError: stray 'G' encountered

which was triggered by this line: #SOURCE:ゲーム「STEINS\;GATE」; in the SM file located at: https://zenius-i-vanisher.com/v5.2/viewsimfile.php?simfileid=45612

It looks like StepMania's MSD parser allows you to use \ to escape any control character (source), so this lib probably should be able to as well.

I... might be able to submit a PR for this sometime in the next few weeks? But I wanted to flag it now, in case someone gets to it first.

There are some idiosyncrasies in how simfiles are handled by StepMania that aren't modeled by this package at all currently. For example, if you want to load the banner for a simfile on disk, you can certainly try using os.path to Frankenstein the simfile's filename and sim.banner together... but that only works if the BANNER tag is filled in, which isn't guaranteed. (For example, quite a few files in UPSMH Online have a blank BANNER tag.) If you leave the field blank, StepMania will try to find the appropriate image file on its own! This also applies to the music path.

While this logic can be implemented in client code, I think it warrants inclusion as part of this package, given that processing simfile data together with audio or image data is a pretty noteworthy use case. It also helps us work toward the goal of handling any simfile that StepMania accepts.

I had an implementation section here previously, but I totally forgot that simfiles don't know their own filenames, so my proposed implementation was bogus. We would likely need a new submodule for loading a simfile's supplementary files.

Keysounded SSC charts store their note data in a NOTES2 property instead of the usual NOTES (the two properties are mutually exclusive to my knowledge). This poses an interesting design problem for simfile: should invoking .notes / ['NOTES'] on an SSCChart use the NOTES2 data when present, or should this logic be kept in NoteData?

Separation of concerns dictates that NoteData should decide where to find the note data, and SSCChart should stay free of "business logic" and act as a plain dictionary of properties. This is how the library works currently, but this functionality isn't particularly discoverable, and I can imagine a lot of users not understanding why they need to call notedata.update_chart(chart) instead of the more straightforward chart.notes = str(notedata). It would be nice if the straightforward solution just worked in all cases.

Any ideas on how to resolve this elegantly?

Thanks for the great package!

I noticed that when parsing charts with 12ths or 24ths, the underlying Fraction representation for Beats are very strange.

Here's a minimal reproducing example where we parse an SM file with a single measure full of 12th notes and print the numerator and denominator of all Note objects:

https://gist.github.com/jcjohnson/603d9b5ab50016429f762cf9a5db2af7

When run, it gives the output:

(0, 1)
(6004799503160661, 18014398509481984)
(6004799503160661, 9007199254740992)
(1, 1)
(6004799503160661, 4503599627370496)
(7505999378950827, 4503599627370496)
(2, 1)
(5254199565265579, 2251799813685248)
(6004799503160661, 2251799813685248)
(3, 1)
(7505999378950827, 2251799813685248)
(8256599316845909, 2251799813685248)

The problem is here: https://github.com/garcia/simfile/blob/master/simfile/notes/init.py#L169

You are passing a float to the Fraction constructor rather than a pair of integers. This will behave fine when the fraction is a power of two (since these can be represented exactly as floating-point values) but will give strange behavior when the fraction is not a power of two.

The fix is simple; you just need to use the Fraction constructor that accepts a pair of integers for the numerator and denominator. When run with --apply-patch=1, my test script above monkey-patches NoteData.__iter__ to apply this fix; this then gives a much more normal result when parsing the chart with 12th notes:

(0, 1)
(1, 3)
(2, 3)
(1, 1)
(4, 3)
(5, 3)
(2, 1)
(7, 3)
(8, 3)
(3, 1)
(10, 3)
(11, 3)

The displaybpm function can't handle #DISPLAYBPM:; or malformed strings currently. I assume StepMania just ignores the field in these cases, but would be good to verify.

Found a couple of asset discovery discrepancies between simfile and StepMania:

• bn.png is discovered by StepMania, but looking at the source code, I don't know why. It seems to look for a filename containing banner or ending with  bn (with a space!), neither of which seem to match bn.png. Maybe FindFirstFilenameContaining is doing something overly complex?
• Matches need to be done case-insensitively (i.e. by converting the filenames being checked to lowercase). I think even assets that are explicitly specified need to be matched case-insensitively (e.g. #BACKGROUND:file.png; should resolve to a FILE.PNG) but it would be good to confirm this.

We should add a stopgap for simfiles with warp events (WARPS tags for SSC, negative BPMs / stops for SM) so that we don't produce invalid simfiles. The lack of a stopgap necessitated a patch in ITL 2022. At minimum we should throw an exception in this scenario.

>>> import simfile
>>> simfileobj = simfile.open('testdata/Springtime.ssc')
>>> simfileobj.charts = []
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: can't set attribute

This really should work, but it doesn't currently. You can modify the charts list, but can't reassign it.

Two bugs in simfile 2.0.0–2.1.0's SSC implementation break multi-value properties, causing them to be truncated or mangled past the first value:

1. When opening an SSC file, the DISPLAYBPM and ATTACKS properties of both simfiles and charts stop parsing at the first :. For DISPLAYBPM, this means a BPM range of min:max will be incorrectly parsed as a static BPM of min. ATTACKS are completely broken as they use colon as a separator.
2. The aforementioned properties are incorrectly serialized with colons escaped from SSCChart. This has the same effects described above, but this only affects values with colons that were written to the chart object manually (e.g. chart.displaybpm = "120:240") since the first bug shadows this bug during deserialization.

Stepmania sorts songs based off of their title transliteration if it exists (it looks like?), so songs with title transliterations aren't sorted by Stepmania in the way this script wants to achieve.

This is my first code contribution to an open source repository, so I may have made some mistakes wrt style/correctness. Maybe a cleaner solution would be to make prefix_title_with_meter title/titletranslit independent and then call it twice? Curious to hear your thoughts :)

Bumps certifi from 2022.6.15 to 2022.12.7.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

We should document simfile.convert with a little more grace than what the AutoAPI page currently has to offer.

The dedicated page should include examples for practical use cases, like saving to both the SM and SSC file in a simfile directory, validating that the two files on disk have matching base properties, and backfilling missing SM files for the sake of StepMania builds that don't support SSC (like NotITG).

The SSC format includes a FAKES property (at both the simfile and chart level) that defines fake regions - beat ranges in which notes of all types are unhittable. This, together with warp segments, are the two reasons why a note might not be hittable, as observed in TimingData.h. (StepMania uses the term judgable here, but hittable appears to be used synonymously elsewhere in source code.)

Right now, TimingEngine only marks warp regions as unhittable. Fixing this cleanly will probably require adding a fakes attribute to TimingData. Currently, the attributes in TimingData are scoped to just those required for TimingEngine to do its job, under which rule this addition makes perfect sense. However, it does count as an expansion to the public API, which will incur a minor version bump.

Also of note: in the future, we may want to add all of the timing properties for which any chart-level data replaces the simfile-level data. This set of properties is documented in NotesLoaderSSC.cpp.

Related to #5. The timing engine should handle negative BPMs as if they were warps. The best way to do this is almost certainly to convert them to warps internally, rather than trying to design the engine's internal data structures around this archaic hack.

Right now TimingEngine is almost certainly prone to raising exceptions if given negative BPMs (I haven't tested it yet). At minimum, it won't correctly flag negative BPM warp regions as being un-hittable(). As such, this qualifies as a bug, although fixing it will likely constitute a feature release (and thus minor version bump) for #5.

StepMania 3.9 infamously "supported" warp segments through a timing data exploit: setting the BPM to negative at some beat, then back to positive at a later beat, would warp the notes from the negative BPM change to... somewhere after the positive BPM change. To my knowledge, this exploit no longer works in StepMania 5, in part because the behavior is now officially supported through the WARPS timing data property. However, the StepMania editor still supports the legacy hack, converting any warps in the internal SSC data to negative BPMs when saving an SM file. This not only means that simfiles with warps created in SM5 will still work in 3.9, but that old files from the 3.9 era can be edited in SM5 non-destructively.

To achieve feature parity, simfile.convert.sm_to_ssc should convert negative BPMs to warps, and simfile.convert.ssc_to_sm should do the opposite (only for simfile warps - warps in the chart timing will raise an exception by default).