-
Notifications
You must be signed in to change notification settings - Fork 64
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
Instanced Projects, AssetLocator, ParamBank, FMGBank #895
Conversation
…p settings in project because looseparams option. Also also it's still used all over and is good for serialization.
…tate in the fmgbank
public static GameType GetGameTypeForExePath(string exePath) | ||
{ | ||
var type = GameType.Undefined; | ||
if (exePath.ToLower().Contains("darksouls.exe")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a static dictionary for a fast lookup? (for all the switches here, might add up if summed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might, but then there's the nuanced cases like des vs bb, or in other such methods the shared cases.
For the whole file, I think ideally we'd define an object/jumptable and do some scummy OOP to handle per-game behaviours (and reuse this "game implementation" class for different databanks eg param loading code, fmg loading code etc.)
But moving to that sort of system might be a step beyond what this PR is focusing on, which is more to instantiate projects and databanks separately, as well as just clean up some asset lookup code.
That is, if I'm understanding your point.
In terms of performance, this particular function is negligibly used.
Instanced FMGBank, more utilities and statics gutted.
FMGBank completely restructured.
Includes merges of assetbrowser but does not yet instance it or move it to new assetlocator utilities