Fix copyright position for PAL 1.1 #2952
Conversation
| G_TX_NOMIRROR | G_TX_CLAMP, G_TX_NOMIRROR | G_TX_CLAMP, G_TX_NOMASK, G_TX_NOMASK, | ||
| G_TX_NOLOD, G_TX_NOLOD); | ||
|
|
||
| gSPTextureRectangle(gfx++, 312, 792, 952, 856, G_TX_RENDERTILE, 0, 0, 1 << 10, 1 << 10); | ||
| gSPTextureRectangle(gfx++, copy_xl, 792, copy_xh, 856, G_TX_RENDERTILE, 0, 0, 1 << 10, 1 << 10); |
There was a problem hiding this comment.
feels like this PR is doing way more than it should, it looks like we already had some logic for 1998 vs 1998/2003, and we just need to set the different locations/lengths here
There was a problem hiding this comment.
It does exactly what is needed, only four values change, and since they only change per version, there is no need to run a function for each value.
| static int EnMag_GetCopyrightTexWidth() { | ||
| uint32_t gameVersion = ResourceMgr_GetGameVersion(0); | ||
|
|
||
| static void EnMag_SetCopyValues(const char** copy_tex, u16* copy_width, u16* copy_xl, u16* copy_xh) { |
There was a problem hiding this comment.
it's not clear why this change is being made instead of utilizing the existing methods
There was a problem hiding this comment.
I had already done this for partial NTSC 1.0 support in the past, I just copied the function for this new version.
There was a problem hiding this comment.
that works, i'm just generally quite risk averse with changes and this is moving us from a pattern that is tested and working to one that i had not seen tested before
There was a problem hiding this comment.
I used the pattern from this example: https://stackoverflow.com/a/9144527
There was a problem hiding this comment.
i understand the pattern, that's not what i was taking issue with. it's the little things like moving to a defining a const char* and passing it by reference being a different pattern than using a returned static char* as a parameter directly. that change switches where in memory that string lives, which may have unintended consequences
There was a problem hiding this comment.
If you think it is convenient, we can use the previous method for this.
There was a problem hiding this comment.
the fact that you've tested it before with partial 1.0 support makes me not worried about it, i just wanted to explain where my hesitation was coming from
Yes. Before: With this PR: |
Build Artifacts