Skip to content
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

common: Copy aglRes-Files from WiiU-Decomp #15

Merged
merged 13 commits into from
Aug 19, 2023

Conversation

MonsterDruide1
Copy link
Contributor

@MonsterDruide1 MonsterDruide1 commented Mar 15, 2023

Due to the amount of stuff needed to get properly defined classes in #13, this has been moved to a separate PR.

This Pull Request mainly copies the files from https://github.com/aboood40091/sead/tree/master/packages/agl/include/common required to add ResShaderArchive and ResShaderBinaryArchive, with minimal changes (formatting to align with our custom guidelines).


This change is Reviewable

@MonsterDruide1 MonsterDruide1 force-pushed the res-shader-stuff branch 2 times, most recently from a61f864 to d7ea8dd Compare March 15, 2023 21:30
@aboood40091
Copy link

IIRC the format changed slightly since NSMBU. You might want to double-check if there are any differences.

Copy link
Contributor

@ThePixelGamer ThePixelGamer left a comment

Choose a reason for hiding this comment

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

I probably missed some things but some more research needs to be done

include/common/aglResBinaryShaderProgram.h Outdated Show resolved Hide resolved
include/common/aglResCommon.h Outdated Show resolved Hide resolved
include/common/aglResCommon.h Outdated Show resolved Hide resolved
include/common/aglResCommon.h Outdated Show resolved Hide resolved
include/common/aglResCommon.h Outdated Show resolved Hide resolved
include/common/aglResShaderMacro.h Outdated Show resolved Hide resolved
include/common/aglResShaderProgram.h Outdated Show resolved Hide resolved
include/common/aglResShaderSource.h Outdated Show resolved Hide resolved
include/common/aglResShaderSymbol.h Outdated Show resolved Hide resolved
include/common/aglResCommon.h Show resolved Hide resolved
@leoetlino
Copy link
Contributor

The assertions can be uncommented - you might need to include sead's assert header first though.

@MonsterDruide1
Copy link
Contributor Author

IIRC the format changed slightly since NSMBU. You might want to double-check if there are any differences.

What files/classes can be looked at here? Searching for agl::Res only gives some Shader-related classes, but it's missing a huge chunk.

include/common/aglResCommon.h Show resolved Hide resolved
include/common/aglResCommon.h Outdated Show resolved Hide resolved
include/common/aglResCommon.h Outdated Show resolved Hide resolved
@leoetlino
Copy link
Contributor

LGTM other than that, thanks for porting these changes over!

include/common/aglResCommon.h Outdated Show resolved Hide resolved
src/utility/common/aglResShaderArchive.cpp Outdated Show resolved Hide resolved
Copy link

@aboood40091 aboood40091 left a comment

Choose a reason for hiding this comment

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

Just need @leoetlino's approval of the non-matching functions in aglResShaderArchive.cpp, the reason for their inclusion being:
#15 (comment)
If he doesn't reply in the upcoming few days, we can merge this ig.

@MonsterDruide1
Copy link
Contributor Author

No response yet, so should we merge without approval, or wait for longer?

Copy link
Contributor

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

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

lgtm (other than some modernisation nits, e.g. xxxxx::iterator could be replaced with auto), sorry for the late review

@leoetlino leoetlino merged commit 611fe02 into open-ead:master Aug 19, 2023
1 check passed
@MonsterDruide1 MonsterDruide1 deleted the res-shader-stuff branch August 19, 2023 08: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.

4 participants