From 1d3056fb1d153778f5fb7d887ce015905434f5fc Mon Sep 17 00:00:00 2001 From: Remis Date: Fri, 6 Nov 2026 18:26:41 +0000 Subject: [PATCH 1/2] mec_load: tolerate blank and NUL-padded state names The 10-byte state-name field is padded, but not always with the same byte: some writers pad with spaces and others with NULs, and older files leave it blank altogether, having never stored the names. mec_load did statename.split()[0], which assumed space padding and at least one token, so a blank field failed in one of two ways depending on how it was padded: space-padded split() returns [], so [0] raised IndexError and the file could not be loaded at all NUL-padded split() does not treat NUL as whitespace, so the name came back as ten NUL bytes - the file loaded, but every state was named out of unprintable characters, which is worse than failing because it is silent Both now resolve: NUL is treated as padding, and a field that is blank after that falls back to a positional name, S1..Skstat, so the states stay distinguishable. Files that do store their names are unaffected. Found on two 2005 AChR mechanism files, one padded each way, where it also left the mechanism's cycle unusable: Cycle holds state names, so nine states named alike cannot define one. With positional names the cycle resolves and checks out at exactly 1.000000 in every record of both files. Regression: 12 .mec files, 351 records, 349 load. The two that do not are records 3 and 22 of QmechRL.mec, which fail differently and already did - their rates refer to state indices beyond the Q matrix allocated from kstat. That is a separate problem and is untouched here. Co-Authored-By: Claude Opus 5 --- scalcs/scalcsio.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/scalcs/scalcsio.py b/scalcs/scalcsio.py index 63c27f1..d8420f4 100644 --- a/scalcs/scalcsio.py +++ b/scalcs/scalcsio.py @@ -650,7 +650,16 @@ def mec_load(mecfile, start): statenames = [] for i in range(0, kstat): statename = f.read(10).decode("utf-8") - statenames.append(statename.split()[0]) + # The 10-byte field is padded, but not always with the same byte: + # some writers pad with spaces and others with NULs. Older files may + # leave it blank altogether, having never stored the names. Treat NUL + # as padding too, and fall back to a positional name, so that a + # mechanism whose names were never written still loads and still has + # distinguishable states. Previously a blank field either raised + # IndexError (space padding, nothing for split to return) or yielded a + # name made of NULs (NUL padding, which split does not strip). + tokens = statename.replace('\x00', ' ').split() + statenames.append(tokens[0] if tokens else 'S{0:d}'.format(i + 1)) #print "State name:", statename #print ("\n") From 858935572b3548d9595dc3a6f199be8cec9f10e4 Mon Sep 17 00:00:00 2001 From: Remis Date: Thu, 12 Nov 2026 18:53:35 +0000 Subject: [PATCH 2/2] Test the reading of .mec state names The previous commit fixed blank and NUL-padded state-name fields in mec_load but had no tests. Its parsing is moved, unchanged, into _state_name(field, number) so that the rule can be tested without a .mec file: - padding is removed whether it is spaces, NULs or both; - no name ever contains a NUL; - a blank field gives S, the state's number in the file - which is also how the drawing stored in the file numbers it, so S4 is the state drawn as 4. Four more tests load real files when SCALCS_MEC_DIR points at a folder of them, and skip otherwise: DREMIS01.MEC, which stores no names; qmechs.mec, whose names are NUL-padded; and qmechsDC1.mec, which names its open states and leaves the shut ones blank. On master, before the fix, all four failed: DREMIS01.MEC loaded with every state named with ten NUL bytes, and so with no layout, since positions keyed by name could not tell the states apart. Co-Authored-By: Claude Opus 5.5 --- scalcs/scalcsio.py | 32 ++++++++++----- tests/test_mec_state_names.py | 77 +++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 tests/test_mec_state_names.py diff --git a/scalcs/scalcsio.py b/scalcs/scalcsio.py index d8420f4..7a851ac 100644 --- a/scalcs/scalcsio.py +++ b/scalcs/scalcsio.py @@ -88,6 +88,26 @@ def _mec_record_to_mechanism(rec): # The mechanism drawing a .mec file carries # --------------------------------------------------------------------------- +def _state_name(field, number): + """The name in one 10-byte state-name field of a .mec file. + + The field is padded, but not always with the same byte: some writers pad + with spaces and others with NULs, and older files leave it blank, never + having stored the names. NUL counts as padding here too, and a field + blank after that gets a positional name, ``S``, so a mechanism + whose names were never written still loads with distinguishable states. + `number` is the state's number in the file - from 1, as the drawing + stored in the file numbers it - so S4 is the state drawn as 4. + + Previously a blank field either raised IndexError (space padding, nothing + for split() to return) or gave a name made of NULs (NUL padding, which + split() does not strip), the second silently: every state of the + mechanism got the same unprintable name. + """ + tokens = field.decode('utf-8').replace('\x00', ' ').split() + return tokens[0] if tokens else 'S{0:d}'.format(number) + + def _layout_from_board(board, statenames): """Positions of the states on the "chess board" stored with a mechanism. @@ -649,17 +669,7 @@ def mec_load(mecfile, start): statenames = [] for i in range(0, kstat): - statename = f.read(10).decode("utf-8") - # The 10-byte field is padded, but not always with the same byte: - # some writers pad with spaces and others with NULs. Older files may - # leave it blank altogether, having never stored the names. Treat NUL - # as padding too, and fall back to a positional name, so that a - # mechanism whose names were never written still loads and still has - # distinguishable states. Previously a blank field either raised - # IndexError (space padding, nothing for split to return) or yielded a - # name made of NULs (NUL padding, which split does not strip). - tokens = statename.replace('\x00', ' ').split() - statenames.append(tokens[0] if tokens else 'S{0:d}'.format(i + 1)) + statenames.append(_state_name(f.read(10), i + 1)) #print "State name:", statename #print ("\n") diff --git a/tests/test_mec_state_names.py b/tests/test_mec_state_names.py new file mode 100644 index 0000000..280b506 --- /dev/null +++ b/tests/test_mec_state_names.py @@ -0,0 +1,77 @@ +"""Tests for reading state names from a .mec file. + +Each name is stored in a 10-byte field. Some writers pad it with spaces and +some with NUL bytes, and older files leave it blank, never having stored the +names at all. A blank field falls back to a positional name, S1, S2, ..., +numbered as the states are in the file - which is also how the drawing stored +in the file numbers them. +""" +import os + +import pytest + +from scalcs import scalcsio + + +class TestStateNameField: + + @pytest.mark.parametrize('field, name', [ + (b'AR* ', 'AR*'), # padded with spaces + (b'AR*\x00\x00\x00\x00\x00\x00\x00', 'AR*'), # padded with NULs + (b'\x00\x00A2R\x00 \x00\x00', 'A2R'), # either, either side + (b'A2R junk ', 'A2R'), # first word only, as before + ]) + def test_padding_of_either_kind_is_removed(self, field, name): + assert scalcsio._state_name(field, 3) == name + + def test_a_field_padded_with_nuls_never_gives_a_name_containing_one(self): + """split() does not count NUL as whitespace, so the old reader named + such states with ten NUL bytes: the file loaded, but every state of + it had the same unprintable name.""" + assert '\x00' not in scalcsio._state_name(b'R' + b'\x00' * 9, 1) + + @pytest.mark.parametrize('field', [b' ' * 10, b'\x00' * 10, b' \x00' * 5]) + def test_a_blank_field_gets_the_state_s_number(self, field): + """Space-padded, the old reader raised IndexError; NUL-padded, it + returned ten NULs.""" + assert scalcsio._state_name(field, 4) == 'S4' + + +MEC_DIR = os.environ.get('SCALCS_MEC_DIR', '') + + +def _load(name): + path = os.path.join(MEC_DIR, name) + if not os.path.isfile(path): + pytest.skip('set SCALCS_MEC_DIR to a folder holding %s' % name) + _, meclist, _ = scalcsio.mec_get_list(path) + return scalcsio.mec_load(path, meclist[0][0]) + + +class TestRealFiles: + """Need .mec files from outside the repository; skipped without them.""" + + def test_a_file_that_stores_no_names(self): + """DREMIS01.MEC: five states, every name field blank.""" + mec = _load('DREMIS01.MEC') + assert sorted(mec.state_names) == ['S1', 'S2', 'S3', 'S4', 'S5'] + + def test_positional_names_follow_the_numbers_on_the_stored_drawing(self): + """DREMIS01.MEC is drawn as C5--C4--C3 + | | + O1--O2 so S5 is top left.""" + layout = _load('DREMIS01.MEC').layout + assert layout['S5'] == (0, 0) + assert layout['S1'] == (2, 2) + + def test_names_padded_with_nuls(self): + """qmechs.mec stores real names, padded with NUL bytes.""" + mec = _load('qmechs.mec') + assert set(mec.state_names) == {'AR*', 'A2R*', 'AR', 'A2R', 'R'} + assert mec.state_index('A2R*') == 1 + + def test_some_names_stored_and_some_blank(self): + """qmechsDC1.mec names its open states and leaves the shut ones + blank.""" + mec = _load('qmechsDC1.mec') + assert set(mec.state_names) == {'AR*', 'A2R*', 'S3', 'S4', 'S5'}