Skip to content

Flatten g_Vram struct to an array of RECT - #3371

Open
bismurphy wants to merge 5 commits into
Xeeynamo:masterfrom
bismurphy:vram_rework
Open

Flatten g_Vram struct to an array of RECT#3371
bismurphy wants to merge 5 commits into
Xeeynamo:masterfrom
bismurphy:vram_rework

Conversation

@bismurphy

Copy link
Copy Markdown
Collaborator

I noticed in game.h we have the Vram struct, which only exists in g_Vram. Every member of the struct is a RECT and they all still have address-based D_whatever names. I think it makes more sense for this to just be an array accessed by index instead of having a dedicated name for each one.

Perhaps it would be useful to have an enum for each index in this array (just like we have for AluTimers), but I think having it as an array rather than a dedicated struct makes more sense here.

@JoshSchreuder

Copy link
Copy Markdown
Collaborator

Seems sensible to me

@Xeeynamo Xeeynamo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue this change is a downgrade. That g_Vram (wrong name, should be g_VramUploadRect) has fixed values through its entire lifecycle.

If we keep using the struct, we could document each field is used for what. For example D_800ACDB8 is for the first chunk of the stage tileset, D_800ACD98 is used for the stage palette, D_800ACDA0 to clear the screen and the stage graphics, and so on.

WDYT?

@bismurphy

Copy link
Copy Markdown
Collaborator Author

I'd argue this change is a downgrade. That g_Vram (wrong name, should be g_VramUploadRect) has fixed values through its entire lifecycle.

If we keep using the struct, we could document each field is used for what. For example D_800ACDB8 is for the first chunk of the stage tileset, D_800ACD98 is used for the stage palette, D_800ACDA0 to clear the screen and the stage graphics, and so on.

WDYT?

By using a system like we have with AluTimers, we can keep the benefits of naming while still avoiding having structs that only exist a single time. In this case, we would do:

enum VramUploadRects{
    VUR_FIRST_STAGETILES,
    VUR_STAGE_PALETTE,
    VUR_CLEAR_SCREEN,
};

and so on.

If this is seen as inferior that's fine, just a suggestion.

@Xeeynamo

Copy link
Copy Markdown
Owner

I'd argue this change is a downgrade. That g_Vram (wrong name, should be g_VramUploadRect) has fixed values through its entire lifecycle.
If we keep using the struct, we could document each field is used for what. For example D_800ACDB8 is for the first chunk of the stage tileset, D_800ACD98 is used for the stage palette, D_800ACDA0 to clear the screen and the stage graphics, and so on.
WDYT?

By using a system like we have with AluTimers, we can keep the benefits of naming while still avoiding having structs that only exist a single time. In this case, we would do:

enum VramUploadRects{
    VUR_FIRST_STAGETILES,
    VUR_STAGE_PALETTE,
    VUR_CLEAR_SCREEN,
};

and so on.

If this is seen as inferior that's fine, just a suggestion.

IMHO I'd prefer g_Vram.firstStageTiles than g_Vram[VUR_FIRST_STAGETILES].

AluTimers is a bit special because it must be accessed through a for loop in EntityAlucard and similar

@bismurphy

Copy link
Copy Markdown
Collaborator Author

I'd argue this change is a downgrade. That g_Vram (wrong name, should be g_VramUploadRect) has fixed values through its entire lifecycle.
If we keep using the struct, we could document each field is used for what. For example D_800ACDB8 is for the first chunk of the stage tileset, D_800ACD98 is used for the stage palette, D_800ACDA0 to clear the screen and the stage graphics, and so on.
WDYT?

By using a system like we have with AluTimers, we can keep the benefits of naming while still avoiding having structs that only exist a single time. In this case, we would do:

enum VramUploadRects{
    VUR_FIRST_STAGETILES,
    VUR_STAGE_PALETTE,
    VUR_CLEAR_SCREEN,
};

and so on.
If this is seen as inferior that's fine, just a suggestion.

IMHO I'd prefer g_Vram.firstStageTiles than g_Vram[VUR_FIRST_STAGETILES].

AluTimers is a bit special because it must be accessed through a for loop in EntityAlucard and similar

Understood, in that case we should create those names like g_Vram.firstStageTiles instead of leaving the g_Vram.D_8675309 values. I don't understand what the Vram struct is doing well enough to do those renamings myself though.

@JoshSchreuder
JoshSchreuder self-requested a review July 28, 2026 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants