Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support LUS::Archive::LoadFileRaw no longer being exposed #3999

Merged
merged 5 commits into from
Mar 6, 2024
Merged

support LUS::Archive::LoadFileRaw no longer being exposed #3999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f826928
tts: use `RawJson` resource + add `SpeechLogger`
briaguya-ai Mar 5, 2024
aa35587
make a branch
briaguya-ai Mar 5, 2024
1d295d8
support `LUS::Archive::LoadFileRaw` no longer being exposed
briaguya-ai Mar 5, 2024
7ca6425
Merge branch 'develop' into rip-out-loadfileraw
briaguya-ai Mar 6, 2024
9131839
latest LUS main
briaguya-ai Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libultraship
Open in desktop
Submodule libultraship updated 5 files
+2 −2 src/resource/archive/Archive.h
+0 −19 src/resource/archive/ArchiveManager.cpp
+0 −2 src/resource/archive/ArchiveManager.h
+2 −1 src/resource/archive/O2rArchive.h
+2 −1 src/resource/archive/OtrArchive.h
2 changes: 1 addition & 1 deletion soh/soh/OTRGlobals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ OTRVersion ReadPortVersionFromOTR(std::string otrPath) {
// Use a temporary archive instance to load the otr and read the version file
auto archive = LUS::OtrArchive(otrPath);
if (archive.Open()) {
auto t = archive.LoadFileRaw("portVersion");
auto t = archive.LoadFile("portVersion", std::make_shared<LUS::ResourceInitData>());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the only SoH side change that isn't part of #3998

Copy link
Contributor

@Archez Archez Mar 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So LoadFileRaw is essentially replaced by LoadFile with a non-null initdata passed in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more or less, if you look at LoadFile in archive now https://github.com/Kenix3/libultraship/blob/d70fd7de2315388c9a99742fd6c634b2cd8be138/src/resource/archive/Archive.cpp#L206-L241 you'll see

    std::shared_ptr<File> fileToLoad = nullptr;

    if (initData != nullptr) {
        fileToLoad = LoadFileRaw(filePath);
        fileToLoad->InitData = initData;
    } else {
        //  [...]
    }

    return fileToLoad;

but using it directly still feels hacky.

i'm also not sure if kenix wants to move LoadFile itself away from being public. if that's the case i'll need to refactor ReadPortVersionFromOTR to use a temporary ResourceManager (which also feels hacky) and that will definitely take a bit more code

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it gets to that point, then we could probably just re-think how portVersion is represented in the archive itself. As long as the update with it lands on a major/minor update then our version detection logic would enforce a regeneration anyways, so the user experience would be the same either way without us having to maintain a deprecated solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it gets to that point, then we could probably just re-think how portVersion is represented in the archive itself.

for sure, it's really just a matter of figuring out how we want to handle needing to read from archives when we don't have a resource manager yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just asked kenix about LoadFile

Public is fine, but you're encouraged to use ResourceManager instead of either ArchiveManager or Archive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is a reasonable way to handle this use case, if we think of a better one we can figure out how we want to implement it, but we definitely won't need to hack together a temporary resourcemanager based solution

if (t != nullptr && t->IsLoaded) {
auto stream = std::make_shared<LUS::MemoryStream>(t->Buffer->data(), t->Buffer->size());
auto reader = std::make_shared<LUS::BinaryReader>(stream);
Expand Down
Loading