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

d_a_tag_myna_light OK #1954

Merged
merged 2 commits into from
Oct 3, 2023
Merged

d_a_tag_myna_light OK #1954

merged 2 commits into from
Oct 3, 2023

Conversation

jakepatzer
Copy link
Contributor

@jakepatzer jakepatzer commented Oct 3, 2023

Resolves #1170

Comment on lines 20 to 23
inline bool checkTurnOnOffChange() {
bool var1 = mTurnOnFlag - field_0x578;
return var1 & 0xFF;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

for inlines defined within the class, you can remove the inline keyword

}

if (field_0x56c >= 0.000001f) {
*(u32*)&color = *(u32*)color_data;
Copy link
Contributor

Choose a reason for hiding this comment

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

can this function be changed to initialize the GXColor here like GXColor color = {0xBC, 0x66, 0x42, 0xFF}? or does it not match like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Huh, I swear I tried this before and it didn't work. Must have been doing something wrong though as it's working now, looks much cleaner!

Comment on lines 147 to 148
static void daTag_MynaLight_Create(daTag_MynaLight_c* i_this) {
i_this->create();
Copy link
Contributor

Choose a reason for hiding this comment

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

this applies to all actor methods (Create, Execute, Draw, Delete, IsDelete), they should all have an int return type and usually return the value of the function its calling. so like here it'd be like

static int daTag_MynaLight_Create(void* i_this) {
    return static_cast<daTag_MynaLight_c*>(i_this)->create();
}

Comment on lines 147 to 148
static void daTag_MynaLight_Create(daTag_MynaLight_c* i_this) {
i_this->create();
Copy link
Contributor

Choose a reason for hiding this comment

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

i notice you changed the parameter types in the method functions, but the name mangling from the symbol map confirms that void* was what was used originally. the symbol map should be followed as closely as possible where possible. in these cases, you can just use a cast to the correct actor type, like
static_cast<ActorType*>(i_this)-> or ((ActorType*)i_this)->

@jakepatzer jakepatzer requested a review from TakaRikka October 3, 2023 07:03
@jakepatzer
Copy link
Contributor Author

(force push above was to amend the first commit with the correct email so it can be linked to my GitHub user)

@TakaRikka TakaRikka merged commit 232af79 into zeldaret:main Oct 3, 2023
1 check passed
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.

d_a_tag_myna_light.cpp
2 participants