-
Notifications
You must be signed in to change notification settings - Fork 17
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
Improves Books by Standardizing "islmDniBook_page/side" #243
base: master
Are you sure you want to change the base?
Conversation
This has now been merged into MOULa; are there any objections to inclusion in this repo? While I understand there were some initial concerns, I believe I have provided sufficient explanation & justification above for why this is the appropriate approach for this enhancement. |
This is on my to-review list. Content PRs are always the worst to review, and this one touches lots of PRPs, so it's very much been on the back-burner. Hopefully I (or someone else) can review the PRC diff before the end of the holiday season. |
I see this currently has a conflict due to a recent change to Kveer_District_Textures.prp that was recently merged as part of #253. I will grab this new version of Kveer_District_Textures and attempt to re-apply the fix in an effort to resolve, but I'd also respectfully request that this PR be given a formal review as soon as possible (once this is resolved) in order to avoid any additional conflicts in the future. Again, note that the equivalent PR has already been accepted in MOULa, and this submission does not violate the content guidelines established for moul-assets. |
Circling back to this one - I get this is a really hard one to evaluate since it touches so many PRPs. I'd be open to radically re-interpreting this one; perhaps breaking it down into a series of much smaller pull requests that are easier to handle. That said - if we can look at the root issue holistically, perhaps we can settle on a specific "if size = x, replace with y" strategy we could then extrapolate to all other examples. In short: the key thing I was trying to address here is (1) many pages textures have ugly yellow splotches left over from an older version of the texture, and (b) many of the book "side" textures are hideously ugly and below a standard that should have been acceptable even back in 2003, let alone 2023. My solution was to make sure every version of this texture in the game was always the same size across the board (which means the biggest one present wins out), but I'm open to a more flexible solution if there are concerns. The main thing is, whatever we agree on, I will want to sync this back into OU, because I really want to prevent these PRPs from diverging any more than they have to. |
I think that's the big hold up here. PRP changes are difficult to review at best, and this big delta is just not something that I've really had a chance to examine from either a visual or technical perspective. Let me find some time for that and get back with you. I'm not sure we need or want to bike shed every single change, but we may want to discuss some things here. |
Totally understood, yeah. Let me go ahead and move this one to a Draft PR instead, and I'll think about different strategies to approach this as well. |
Equivalent to MOULa content/138. Improves appearance of books in base game by ensuring all references to "islmDniBook_page" and "islmDniBook_side" are consistent in all PRPs. This is a 1-1 texture improvement for which there is already precedent in-game and does not introduce anything new, rather just updates all PRPs for consistency.
The equivalent content/138 is included in MOULa Q32023 and has been thus far well-received, but I will walk back the changes on OU if there are significant concerns over accepting this into H'uru, as I would strongly prefer these stay consistent and not diverge. For this PR, the changes have been recreated from scratch for H'uru, building on top of H'uru's most recent PRPs (and making the spine and page changes separate commits). All affected ages have been tested in the H'uru client where possible, and no apparent LOC issues have been identified.
I stand by the approach here: it looks good, behaves well, and has minimal impact.
As there were previously some initial concerns over these sizes seeming unusual for these textures, as well as some hesitation over the precedent an update like this might set, let me be very clear about what this update DOES and DOES NOT do:
What this update DOES do:
xbooknewpage
) already in use in ahnonaycathedral & greatTreePub.What this update DOES NOT do:
Other Important Notes:
*0#0
(or blank) and does not correlate in any meaningful way with size, which is seemingly random. These are thus the most inconsistent and most in need of a refresh here.*0#0
indicates original size,*0#2
indicates "medium" size,*0#3
indicates smallest size was used.Page
" and "Page1
" are considered DIFFERENT textures - you will note there are TWO source textures for this provided from Intangibles, even though these images are identical. The reason for this is at lower resolutions, these "become" different, distinct textures - "page1
" is smoother, while the other, "page
" is more detailed & grainy - essentially, different compression methods. But these "collapse" down into the same texture at standard resolution. Being intentional, this should be left alone and not considered duplicate textures. If, hypothetically, someone wanted to create variants of these textures that replaced all instances of "page
" with a sligtly darker & granier variant to make this more accurate to the lower-res texture, they are welcome to do so, but it is not in the scope of what I was looking to accomplish here - nor is it consistent with how "newer" books that simply use two instances ofxbooknewpage
look.Hypothetical next steps for a followup project:
page
&page1
s COULD be re-namedislmdnibook_page*0#0.hsm
&islmdnibook_page1*0#0.hsm
(orislmDniBook_page.dds
&islmDniBook_page1.dds
) - however, leaving these alone may also be useful, as indications of the original sizes may be helpful reference.See attached txt for the full list of PRPs & files this applies to.
Affected_textures.txt