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

Starfield #130

Merged
merged 5 commits into from
Sep 17, 2023
Merged

Starfield #130

merged 5 commits into from
Sep 17, 2023

Conversation

Silarn
Copy link
Member

@Silarn Silarn commented Sep 6, 2023

Add interfaces for the new 'secondaryDataDirectories' and 'overridePluginsAreSupported' methods needed to account for the Starfield quirks.

*
* @return directories where we may find data files outside the main location
*/
virtual QMap<QString, QDir> secondaryDataDirectories() const = 0;
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't really feel like it's something that we can reuse for many games. Probably only for starfield. As such I would suggest we put this into a GameFeature so we don't pollute the main IPluginGame interface.

I think it might've been cleaner to add this way too.

Copy link
Member

Choose a reason for hiding this comment

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

I don't really mind the extra functions for IGamePlugin, moving it to a game feature seems overkill for a single function. But I think this should be defaulted instead of pure-virtual.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, then we can keep it like this. I would consider moving this to a dedicated feature if we find we need to expand this with more methods in the future.

Copy link
Member

Choose a reason for hiding this comment

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

I also think this should return an std::map instead of a QMap. Most recent changes to uibase use standard containers instead of Qt ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could replace the original function, but then we need to adjust every game plugin. This still provides functionality that could be potentially useful to other games without requiring updates to every other game plugin. Though I suppose you could deprecate it and have gamebryo handle converting the old method to a new map.

In any case, it can be updated in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Sometimes games have their own VFS, so long-term, it could be nice if we had something both for dealing with that and with Starfield. It would probably be reasonable to move to a dedicated game feature if we implement support for mutable VFSes, i.e. where we can install things with them instead of using USVFS.

- Primary plugins are still marked as loaded
@Silarn Silarn merged commit b88ff9c into master Sep 17, 2023
3 checks passed
@Silarn Silarn deleted the starfield branch October 4, 2023 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants