Support RomFS mode for SR

[ThanatosGit]

Kinda "mixed mode" ? Because we need to keep "_discardables.pkg".

[ThanatosGit]

Was there a specific purpose for this, I don't see?
Just to clarifiy why I had to remove it so far:
The issue here with SR is that some asset are in asset_ids_to_copy because they are ensured (see L338-L341).
Deleting some of them doesn't work because we need to keep _disardables.pkg files, that would throw a key error in L396 if they are removed.
So, the use case is: Ensured assets which needs to be written to the discardables.pkg

[henriquegemignani]

ugh there was a reason. Struggling to remember why though.

What comes to mind is the need to keep things in system.pkg even if output being ROMFS?

[ThanatosGit]

What comes to mind is the need to keep things in system.pkg even if output being ROMFS?

Hmmm isn't that the same case I have in SR with keeping _discardables.pkg? I think, the current upstream code should also throw a key error if you ensure something into the system.pkg for Dread because the lines in question here, would delete the asset from the asset_ids_to_copy.

I only see a memory save thing here? If you ensure something into a pkg but it's already written to the RomFS by this code, you don't need to write it to the pkg later again. (Of course that's only true for non system.pkg in dread).

[ThanatosGit]

I think, the current upstream code should also throw a key error if you ensure something into the system.pkg for Dread because the lines in question here, would delete the asset from the asset_ids_to_copy.

Isn't true. That is really some piece of code :D.
The loop around it is only executed for _modified_resources and the if around the del checks if it is in any pkg.

E.g. to get a KeyError: 15223854729795290759 in dread with current upstream MEDS, you can take a resource from a pkg, modify it and then ensure it into one of the files which still have to be written as a pkg (so in Dread only system.pkg)

    bla = editor.get_file("actors/characters/iceflea/charclasses/iceflea.bmsad")
    bla.raw["name"] = "abc"
    editor.ensure_present("packs/system/system.pkg", "actors/characters/iceflea/charclasses/iceflea.bmsad")

Leads to a KeyError because the del is executed.

Though I'm not brain enough today to get the reason for the delete.

[dyceron]

Couldn't you just slap a and if game ~= Game.SAMUS_RETURNS instead of removing it, since it sounds like Dread does use it? I could be mistaken though and not following along.

[ThanatosGit]

That's the safety method, yeah. Guess I use it then to may not break dread by accident even I think I understood what this is doing.

The whole thing could benefit from a refactor which would be like: While iterating through modified ressources, you only want to delete from asset_ids_to_copy if the asset_id is not ensured in a pkg from modified_pkgs.

Changed the commit and also will update the PR title to be correct

[codecov]

Codecov Report

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

Project coverage is 74.39%. Comparing base (60ca7a3) to head (46ecc72).

Files	Patch %	Lines
...mercury_engine_data_structures/file_tree_editor.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #141   +/-   ##
=======================================
  Coverage   74.39%   74.39%           
=======================================
  Files          63       63           
  Lines        3199     3199           
=======================================
  Hits         2380     2380           
  Misses        819      819           

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

[dyceron]

This honestly seems fine given it doesn't interfere with dread anymore, and keeps the discardables thing relevant