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.

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

VideoCommon: add an asset library that loads directly from the filesystem #11876

Merged

Conversation

iwubcode
Copy link
Contributor

@iwubcode iwubcode commented Jun 2, 2023

Built on top of #11875

@iwubcode iwubcode force-pushed the directfilesystem_asset_library branch from e8a00dc to 07307ed Compare June 2, 2023 19:49
Comment on lines +29 to +30
void SetAssetIDMapData(const AssetID& asset_id,
std::map<std::string, std::filesystem::path> asset_path_map);
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I realize this has now resulted from my note about the asset_id being a file system path, but this is... pretty strange to me. Does that mean you now have to 'prime' every asset you load before you load it by calling this function? How would this work in a situation where you don't know (or care) what library is hiding behind your CustomAssetLibrary pointer you have?

In fact, okay, maybe step back. Can you give me a little example (pseudocode is fine) on how exactly the general flow here is supposed to work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does that mean you now have to 'prime' every asset you load before you load it by calling this function?

Yes.

Examples.

Graphics mods:

Plan is to have a blob in the json called "assets" with the mapping from name (assetID) to path. Eventually an editor would probably define the name, maybe as a GUID (I think this is what Unity does).

Texture packs:

You'd iterate the filesystem on load, looking for the files. Then have the textureid as the assetID and map that to the path on the disk.


A really dumb mapper might be like:

for (const auto& file_path : hires_texture_paths)
{
  std::map<std::string, std::filesystem::path> asset_mapper;
  // The map usage is if we had multiple files we wanted to map to one asset
  // we only have one file, so just make a dummy key, it doesn't matter in the texture loading code
  asset_mapper[""] = file_path;
  m_directfilesystem_library->SetAssetIDMapData(file_path.name(), std::move(asset_mapper);
}

I'll admit it is a bit klunky but after your comment, I realized a filesystem path doesn't make a lot of sense either. There's times we don't know the path!

This might be something we have to grow into after we get all the cases laid out (I haven't finished designing material/shader assets yet). I am open to ideas.

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Jun 2, 2023 via email

@AdmiralCurtiss AdmiralCurtiss merged commit 4ee2740 into dolphin-emu:master Jun 2, 2023
14 checks passed
@iwubcode iwubcode deleted the directfilesystem_asset_library branch June 2, 2023 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants