Add "More info in file select" enhancement #3053

Pepe20129 · 2023-07-01T17:09:00Z

Adds the "More info in file select" enhancement which shows the items that you've obtained similarly to how n64 randomizer does it.

Here is a 100% vanilla save file:

Here is a new vanilla save file:

Here is a new randomizer save file:

This doesn't apply when copying or erasing save files:

This PR also changes the spoiler log seed hash icons to only appear on quest selection and the file hash icons to appear at the top when a randomizer save file is selected.

PurpleHato · 2023-07-01T18:35:29Z

Visually, I have a tiny nitpick for the double defence going vertically instead of horizontally, which makes them overlapping the FILE title holder
If you're worrying about the centering of it, you probably can do a very simple conditional to move them accordingly if being double defence or not

Pepe20129 · 2023-07-01T18:38:46Z

The reason it goes vertically instead of horizontally is that the numbers of the counter are tied to the icon, so if I shifted it horizontally the numbers would follow one heart instead of being in the middle of them. Also the positions of icons are constant in the current system so that's another reason why I chose to have them vertically (the top one appears or disappears and the bottom one is always there).

PurpleHato · 2023-07-01T18:42:57Z

The reason it goes vertically instead of horizontally is that the numbers of the counter are tied to the icon, so if I shifted it horizontally the numbers would follow one heart instead of being in the middle of them. Also the positions of icons are constant in the current system so that's another reason why I chose to have them vertically (the top one appears or disappears and the bottom one is always there).

I see, I wonder if it would be possible to create a "dummy" icon that would be hidden and the size of the double defence one which will be used for the number tracking and basically overlay itself at the center of your hearts display?

I'm just throwing some idea on top of my head right now

Pepe20129 · 2023-07-01T18:46:57Z

The only way that'd work would be with 4 icons; the dummy one, the normal hearts one and one for each of the double defense hearts.

I could move the double defense icon slightly down so it doesn't overlay the FILE box as much.

Archez

Overall the implementation looks clean and the patterns used are easy to follow. Left some minor comments and suggestions below.

Archez · 2023-07-03T14:39:43Z

soh/soh/SohMenuBar.cpp

@@ -956,6 +956,8 @@ void DrawEnhancementsMenu() {
                                "This might affect other decal effects\n");
            UIWidgets::PaddedEnhancementSliderInt("Text Spacing: %d", "##TEXTSPACING", "gTextSpacing", 4, 6, "", 6, true, true, true);
            UIWidgets::Tooltip("Space between text characters (useful for HD font textures)");
+            UIWidgets::PaddedEnhancementCheckbox("More info in file select", "gFileSelectMoreInfo", true, false);


I wonder if this should move to the Gameplay submenu instead. Although it renders icons, it doesn't really feel like a graphics option.

Even though I'm still not a fan of it's current location, the randomizer enhancements submenu is probably better for this. It should also be enabled by default similar to the other randomizer enhancements

It's not a randomizer enhancemet specifically, it works with vanilla and mq save files

The feature is not limited to randomizer saves, so that may be confusing. Unless we would rather it be forced on for rando saves, and instead the toggle is only used for vanilla/mq saves?

I feel like that would also be weird

Oh nice, I guess I hadn't considered it being useful outside of rando. disregard my comment then. Default on for rando would be nice to be consistent with zootr

soh/src/overlays/gamestates/ovl_file_choose/z_file_choose.c

garrettjoecox

Code is very clean great work!

Only minor feedback I have is we can probably remove the != 0; and == 0 for ints that are effectively bools, especially when the function name makes the return type obvious (HasItem and ShouldRenderItem)

inspectredc · 2023-07-03T16:23:27Z

Code is very clean great work!

Only minor feedback I have is we can probably remove the != 0; and == 0 for ints that are effectively bools, especially when the function name makes the return type obvious (HasItem and ShouldRenderItem)

I believe they did that because the return type is a different size to the evaluated return statement. I think it would be better to either shift the bits right rather than the flag to the left to effectively make it a bool or just leave it as is since the item tracker apparently does this anyway.

Pepe20129 · 2023-07-03T16:26:26Z

Only minor feedback I have is we can probably remove the != 0; and == 0 for ints that are effectively bools, especially when the function name makes the return type obvious (HasItem and ShouldRenderItem)

The reason some of these were added was because when checking for certain values, like quest items with an index over 7, the value would be truncated and would return 0 even if you had the item.

garrettjoecox · 2023-07-03T16:29:14Z

The reason some of these were added was because when checking for certain values, like quest items with an index over 7, the value would be truncated and would return 0 even if you had the item.

In this case that makes sense:

return (Save_GetSaveMetaInfo(fileIndex)->questItems & (1 << (item - 0x66))) != 0;

But not so much in these cases

HasItem(fileIndex, ITEM_MASK_GORON) == 0
ShouldRenderItem(fileIndex, data->item) != 0
CVarGetInteger("gFileSelectMoreInfo", 0) == 0

Pepe20129 · 2023-07-03T16:42:25Z

Removed the unneeded checks

Malkierian · 2023-07-12T17:56:17Z

Just a thought, have you considered checking if this works with texture packs? We do still have the bugged song display in the item tracker when high-res textures are used, and figured either that would also apply here and should be looked into, or this already accounts for it and the process could be applied to the item tracker.

Pepe20129 · 2023-07-12T18:43:40Z

I have not checked it with texture packs, I assume it's also broken. I did notice that the textures that are broken on the check tracker are the ones in ia8 format, so it's probably a bug in the decoding of ia8 textures or packs saving them in the wrong format (rgba instead of ia).

Malkierian · 2023-07-12T23:58:50Z

OK, so Archez actually just mentioned he figured out the song icons thing, so you shouldn't have to worry.

garrettjoecox · 2023-08-07T02:00:22Z

Marking this as merge ready, will let it simmer for a bit longer before merging

Archez

LGTM
Only recommendation I have based on earlier conversation is maybe we can change this from a toggle to a dropdown similar to "Allow cursor over empty slots on pause screen", where there is a Never, Only for Rando Saves and Always option.
But that doesn't need to block this PR as is, and can happen in a follow up PR if others agree.

Pepe20129 · 2023-08-11T20:27:55Z

I don't see why someone would want this but only for rando save files, that said, I can add it if others want it

briaguya-ai · 2023-08-11T20:31:36Z

are the screenshots up to date? the first one has a double heart thing going on

Pepe20129 · 2023-08-11T20:35:06Z

yes, having two hearts represents double defense, in n64 rando they're next to each other horizontally instead of vertically, the problem was explained before:

The reason it goes vertically instead of horizontally is that the numbers of the counter are tied to the icon, so if I shifted it horizontally the numbers would follow one heart instead of being in the middle of them. Also the positions of icons are constant in the current system so that's another reason why I chose to have them vertically (the top one appears or disappears and the bottom one is always there).

briaguya-ai

LGTM. I'm not the biggest fan of the stacked hearts for double defense, but that definitely shouldn't block merging. One little nit about abbreviated macro names but that also shouldn't block merging.

briaguya-ai · 2023-08-11T20:40:50Z

soh/src/overlays/gamestates/ovl_file_choose/z_file_choose.c

+#define INV_IC_POS(x, y) {0x4E + ICON_SIZE * x, 0x00 + ICON_SIZE * y}
+#define EQP_IC_POS(x, y) {0x7E + ICON_SIZE * x, 0x2A + ICON_SIZE * y}
+#define SNG_IC_POS(x, y) {0x49 + SONG_WIDTH * x, 0x45 + SONG_HEIGHT * y}
+#define UPG_IC_POS(x, y) {0x5A + ICON_SIZE * x, 0x2A + ICON_SIZE * y}
+#define STN_IC_POS(i) {0x29 + ICON_SIZE * i, 0x31}


nit: it'd be nice to have these named without abbreviations

https://www.youtube.com/watch?v=-J3wNP6u5YU&t=68s

Pepe20129 · 2023-08-11T20:46:55Z

I do plan on doing a follow up pr once #3062, #3082 & #3099 are merged to add support for them, I can change the macro names and try to improve the double defense icon in that pr.

Pepe20129 added 15 commits June 29, 2023 19:08

Initial attempt

0089b7a

Fix rendering for most items

95dc99b

Fixed icon sizes and reorganized icons

a6021b3

Fix equipment, quest & upgrade icons

b4b92a6

Add colors to the song icons

004350a

Remove box, clean up code & move seed hash icons to top

7bfe97f

Start with counters, fix copy & erase and show spoiler log hash icons

c9d17ff

Add icons for upgrades

b5e2cb9

Draw icons for counters

3933aa4

Initial counter work

833ef41

Fix counter digits positioning

6bd5e6d

Prevent crashes when over 999 deaths (save editor)

6be4ee1

Add greg to tracker when in a rando save

bbb6485

Fix color for counter digits using the wrong buffer

5e9b89b

Add double defense icon

6d176b0

Archez reviewed Jul 3, 2023

View reviewed changes

garrettjoecox reviewed Jul 3, 2023

View reviewed changes

soh/src/overlays/gamestates/ovl_file_choose/z_file_choose.c Outdated Show resolved Hide resolved

Addressed code review

8395255

garrettjoecox approved these changes Jul 3, 2023

View reviewed changes

Remove unneeded checks against 0

8a037ce

Pepe20129 mentioned this pull request Jul 10, 2023

Rando: Shuffle Child's Wallet #3075

Closed

garrettjoecox added the merge ready label Aug 7, 2023

Archez approved these changes Aug 11, 2023

View reviewed changes

garrettjoecox added merge ready and removed merge ready labels Aug 11, 2023

briaguya-ai approved these changes Aug 11, 2023

View reviewed changes

garrettjoecox merged commit 8872a59 into HarbourMasters:develop Aug 16, 2023
1 check passed

Pepe20129 deleted the more_file_select_info branch August 16, 2023 00:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "More info in file select" enhancement #3053

Add "More info in file select" enhancement #3053

Pepe20129 commented Jul 1, 2023

PurpleHato commented Jul 1, 2023 •

edited

Pepe20129 commented Jul 1, 2023

PurpleHato commented Jul 1, 2023

Pepe20129 commented Jul 1, 2023

Archez left a comment

Archez Jul 3, 2023

garrettjoecox Jul 3, 2023

Pepe20129 Jul 3, 2023

Archez Jul 3, 2023

Pepe20129 Jul 3, 2023

garrettjoecox Jul 3, 2023

garrettjoecox left a comment

inspectredc commented Jul 3, 2023

Pepe20129 commented Jul 3, 2023

garrettjoecox commented Jul 3, 2023

Pepe20129 commented Jul 3, 2023

Malkierian commented Jul 12, 2023

Pepe20129 commented Jul 12, 2023

Malkierian commented Jul 12, 2023

garrettjoecox commented Aug 7, 2023

Archez left a comment •

edited

Pepe20129 commented Aug 11, 2023 •

edited

briaguya-ai commented Aug 11, 2023

Pepe20129 commented Aug 11, 2023

briaguya-ai left a comment

briaguya-ai Aug 11, 2023

Pepe20129 commented Aug 11, 2023 •

edited

Add "More info in file select" enhancement #3053

Add "More info in file select" enhancement #3053

Conversation

Pepe20129 commented Jul 1, 2023

PurpleHato commented Jul 1, 2023 • edited

Pepe20129 commented Jul 1, 2023

PurpleHato commented Jul 1, 2023

Pepe20129 commented Jul 1, 2023

Archez left a comment

Choose a reason for hiding this comment

Archez Jul 3, 2023

Choose a reason for hiding this comment

garrettjoecox Jul 3, 2023

Choose a reason for hiding this comment

Pepe20129 Jul 3, 2023

Choose a reason for hiding this comment

Archez Jul 3, 2023

Choose a reason for hiding this comment

Pepe20129 Jul 3, 2023

Choose a reason for hiding this comment

garrettjoecox Jul 3, 2023

Choose a reason for hiding this comment

garrettjoecox left a comment

Choose a reason for hiding this comment

inspectredc commented Jul 3, 2023

Pepe20129 commented Jul 3, 2023

garrettjoecox commented Jul 3, 2023

Pepe20129 commented Jul 3, 2023

Malkierian commented Jul 12, 2023

Pepe20129 commented Jul 12, 2023

Malkierian commented Jul 12, 2023

garrettjoecox commented Aug 7, 2023

Archez left a comment • edited

Choose a reason for hiding this comment

Pepe20129 commented Aug 11, 2023 • edited

briaguya-ai commented Aug 11, 2023

Pepe20129 commented Aug 11, 2023

briaguya-ai left a comment

Choose a reason for hiding this comment

briaguya-ai Aug 11, 2023

Choose a reason for hiding this comment

Pepe20129 commented Aug 11, 2023 • edited

PurpleHato commented Jul 1, 2023 •

edited

Archez left a comment •

edited

Pepe20129 commented Aug 11, 2023 •

edited

Pepe20129 commented Aug 11, 2023 •

edited