-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Spd/Spr Merging #10
Spd/Spr Merging #10
Conversation
sprite modifications are now held in a separate dictionary and patched in right before the emulated spd is written
should fix emulated sprites crashing when viewing in amicitia
texture replacement is now just sprite replacement but applied to every sprite that pointed to the texture. also added the ability to exclude sprites from this as well
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.
Some Misc Questions
What is the Use Case for Adding New Sprites?
i.e. How are the sprites referenced. Are they referenced in scripts? (.flow, etc.)
I ask because if the user adds a sprite in order to add something that was not in the original game, and another mod also uses that same sprite index to add its own new sprite, that should ideally be resolved, so both mods can get their 'new' features.
Do we have the capabilities to resolve these conflicts down the road? I think it's possible, but I need to ensure it is.
Can this be a Stream rather than a File Emulator
We're currently parsing out the archive, and building a new archive in memory, keeping all of the new data in RAM. When textures are involved, this is not ideal; as textures are generally a large memory hog, especially since mod authors in games have a habit of throwing Super Ultra Mega 4K texture packs without consideration of how the asset is displayed on the screen.
This means they sometimes wind up making textures at mixed resolutions, 8K, 4K, 16K screen etc. Risking storing 100s of MB in RAM is not ideal.
In the case of BF Emulator, which this code seems inspired by, it wasn't very possible as data from the original file wasn't reusable; however the SPD(s) are essentially archive files, without any sort of SOLID compression, etc.
For the texture data, however, you should access the original textures, and the new textures via a FileSliceStream
, such as FileSliceStreamW32
; such that they are loaded from disk, and not from memory at runtime. (see: AFS Redirector and other Stream Based Redirectors)
Unit Tests Requested
Unit tests for the new Emulator should ideally be added. I'm surprised you didn't, as it's faster to test emus via unit testing rather than directly booting the game.
Separation of Concerns / 'Pay for What You Need'
When it's ported to Rust, circa late 2024, SPD and SPR Emulator should ideally be split. I have a very strong belief that end users should only pay for what they use; so code for SPR if game only uses SPD isn't very useful, and vice versa.
For now this is fine, but for R3, Persona Essentials itself will likely be split per game, and transitive dependencies like SPR/SPD emulator would only be enabled if any mod that depends on essentials uses them. i.e. a mod will be able to specify a dependency on another mod without automatically enabling that mod
What is the Use Case for Adding New Sprites?i.e. How are the sprites referenced. Are they referenced in scripts? (.flow, etc.)Currently the only way sprites are referenced is in the executable, unless you're utilizing my unhardcoded toolkit mod. In terms of potential conflicts for new sprites, If two mods add the same new sprite id, one will overwrite the other, there's no way to resolve that in code that I can think of... Can this be a Stream rather than a File Emulatoryea probably :P I'll look into converting it Unit Tests Requested:hee_melt: (It somehow didn't register to me that there was a test project) |
I have the tests all commented out and I didn't add the spd emulator as a project reference because doing so would require updating reloaded.memory to the newest version. The problem with this is it means every dependency needs to be updated, and the dependencies for the dependencies need to be updated, and code needs to be changed because of changed/removed methods. I spent about an hour trying to do it and gave up :/
Everything you mentioned should be addressed (kinda, read my commit on the tests), hopefully I correctly understood everything you wanted me to do 🥴 |
…t doesn't seem to display in the warning "window"
the builder will now only read and replace sprite entry data when processing spr_ textures. This had to be done because of the new texture replacement system completely negating sprite entry changes
put one of the paths in a codeblock for consistency
forgot to change some of the ones I copypasted from bf
No description provided.