BMSSD refactor #226

steven11sjf · 2024-10-04T05:37:35Z

now uses an adapter to place blocks, objects and lights in hidden keys and puts the objects in the scene group data
rebuilds all but 4 msr files with missing crc values
added function to get a block or light by name
added function to get a scene group
added function to find scene groups an item belongs to
added function to remove an item from a scene group
added function to add an item
added function to remove an item
beeg test function

screenshot of sample --dump-to output. functions should be able to do anything user would want to without requiring manipulating raw input.

codecov · 2024-10-04T05:46:21Z

Codecov Report

Attention: Patch coverage is 97.95918% with 2 lines in your changes missing coverage. Please review.

Project coverage is 76.84%. Comparing base (ef9f5ad) to head (31cfd22).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...rc/mercury_engine_data_structures/formats/bmssd.py	97.93%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #226      +/-   ##
==========================================
+ Coverage   76.33%   76.84%   +0.51%     
==========================================
  Files          78       78              
  Lines        3782     3874      +92     
==========================================
+ Hits         2887     2977      +90     
- Misses        895      897       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ThanatosGit · 2024-10-04T08:03:51Z

@dyceron Whenever this gets merged, someone of us needs to change the disable_vignettes function because this PR breaks it.
We have to remove the vignette from the scene_groups -> collision camera name -> find the model_name and remove it. Except for s090, where it looks like we also remove from objects and not only from scene_groups.
So, it needs some changes.

ThanatosGit · 2024-10-04T10:45:26Z

tests/formats/test_bmssd.py

+sr_xfail = [
+    "maps/levels/c10_samus/s000_surface/s000_surface.bmssd",
+    "maps/levels/c10_samus/s020_area2/s020_area2.bmssd",
+    "maps/levels/c10_samus/s050_area5/s050_area5.bmssd",
+    "maps/levels/c10_samus/s110_surfaceb/s110_surfaceb.bmssd",
+]
+


I'm not sure if I like that. Atm we have to parse s010, s030, s070 and s090 for osrr. Luckily, none of them are in this list but this list still means we are able to parse less files compared to before.

I can look into this further. It's possible that those keys are assets removed from the bmssd, or possibly in another format like bmssa that isn't documented. I can maybe make it store the integers as a new "missing asset" type or remove them when parsing if it doesn't affect stability.

I mostly am looking for feedback on the API changes so it's good to see SR has actual use cases currently.

duncathan

would love if Than can take a look at this and give the OK for MSR purposes

duncathan · 2024-10-29T22:20:00Z

src/mercury_engine_data_structures/formats/bmssd.py

+from mercury_engine_data_structures.common_types import CVector3D, StrId, VersionAdapter, make_dict, make_vector
+from mercury_engine_data_structures.crc import crc32, crc64
+
+TransformStruct = Struct("position" / CVector3D, "rotation" / CVector3D, "scale" / CVector3D)


honestly, throw this in common_types and just call it Transform3D or smth. that way if we want we could optimize it in one place, and I expect it'll have uses in other formats

duncathan · 2024-10-29T22:24:36Z

src/mercury_engine_data_structures/formats/bmssd.py

+        1: "objects",
+        2: "lights",
+    }
+


surely you can use the ItemType enum for whatever this is doing?

I realized just now that the way i resolved this (using the enums as dict keys) breaks dumping json through cli. should I make it use strings to prevent this?

Could possibly make it a IntEnum (or a StrEnum which is better for dumping but requires py3.11)

class ItemType(str, Enum) will do anything you may need StrEnum for

about that, lmao.

https://tomwojcik.com/posts/2023-01-02/python-311-str-enum-breaking-change

i think IntEnum would still work but eugh, StrEnum would be so much nicer.

ok, so override __format__() if you need to guarantee specific behavior from it? I don't really see what the problem is

- now uses an adapter to place blocks, objects and lights in hidden keys and puts the objects in the scene group data - rebuilds all but 4 msr files with missing crc values - added function to get a block or light by name - added function to get a scene group - added function to find scene groups an item belongs to - added function to remove an item from a scene group - added function to add an item - added function to remove an item - beeg test function

- refactored get_item to allow getting any item by crc (or index for objects) - updated test

for more information, see https://pre-commit.ci

- moved scene_groups.item_count calculation into BmssdAdapter.encode - removed lambda from scene_groups.item_count and compiled BMSSD - bmssd test duration -- before: 27.34s, after: 21.36s

- moved TransformStruct to common types as Transform3D - used ItemType enum more consistently - fixed tests to use new ItemType format

- todo: just use strenum after we stop supporting 3.10

steven11sjf requested review from henriquegemignani, duncathan and MayberryZoom October 4, 2024 05:37

steven11sjf added game: Metroid: Samus Returns game: Metroid Dread labels Oct 4, 2024

ThanatosGit reviewed Oct 4, 2024

View reviewed changes

steven11sjf force-pushed the bmssd-refactor branch from 1664f84 to 6dc9869 Compare October 17, 2024 21:01

duncathan requested changes Oct 29, 2024

View reviewed changes

duncathan requested a review from ThanatosGit October 29, 2024 22:26

steven11sjf and others added 7 commits October 30, 2024 14:37

BMSSD

45e635a

- refactored get_item to allow getting any item by crc (or index for objects) - updated test

[pre-commit.ci] auto fixes from pre-commit.com hooks

677edce

for more information, see https://pre-commit.ci

adjusted tests

4119b78

fixed ruff

97121ce

make bmssd compilable

b52b1b6

- moved scene_groups.item_count calculation into BmssdAdapter.encode - removed lambda from scene_groups.item_count and compiled BMSSD - bmssd test duration -- before: 27.34s, after: 21.36s

dunc review

2d9c803

- moved TransformStruct to common types as Transform3D - used ItemType enum more consistently - fixed tests to use new ItemType format

steven11sjf force-pushed the bmssd-refactor branch from 9b074f4 to 2d9c803 Compare October 30, 2024 19:37

uses str enum

31cfd22

- todo: just use strenum after we stop supporting 3.10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BMSSD refactor #226

BMSSD refactor #226

steven11sjf commented Oct 4, 2024

codecov bot commented Oct 4, 2024 •

edited

Loading

ThanatosGit commented Oct 4, 2024

ThanatosGit Oct 4, 2024

steven11sjf Oct 4, 2024

duncathan left a comment

duncathan Oct 29, 2024

duncathan Oct 29, 2024

steven11sjf Nov 4, 2024

steven11sjf Nov 4, 2024

duncathan Nov 4, 2024

steven11sjf Nov 10, 2024

duncathan Nov 10, 2024

BMSSD refactor #226

Are you sure you want to change the base?

BMSSD refactor #226

Conversation

steven11sjf commented Oct 4, 2024

codecov bot commented Oct 4, 2024 • edited Loading

Codecov Report

ThanatosGit commented Oct 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duncathan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 4, 2024 •

edited

Loading