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

Matched 2.0I and 2.0J, fixed matched object count calculation #53

Merged
merged 6 commits into from
Sep 3, 2023

Conversation

Mr-Wiseguy
Copy link
Collaborator

No description provided.

Makefile.ido Outdated Show resolved Hide resolved
src/os/getmemsize.c Outdated Show resolved Hide resolved
src/io/pfssetlabel.c Outdated Show resolved Hide resolved
src/io/pfsrepairid.c Outdated Show resolved Hide resolved
OSPiHandle *__osDiskHandle;

OSPiHandle *osLeoDiskInit() {
u32 saveMask;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I think some of the code files you added used tabs intsead of 4 spaces. Not sure where they all are.

Copy link
Contributor

@hensldm hensldm left a comment

Choose a reason for hiding this comment

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

Still seeing tabs in places. We may want to run a formatter.

include/PR/os_motor.h Outdated Show resolved Hide resolved
src/audio/synstartvoiceparam.c Outdated Show resolved Hide resolved
src/io/cartrominit.c Outdated Show resolved Hide resolved
src/io/conteepread.c Outdated Show resolved Hide resolved
@Mr-Wiseguy
Copy link
Collaborator Author

I do think a formatter would be a good idea, but there are a lot of files in this codebase that already have tab indentation and changing those would be outside the scope of this PR.

#ifdef _DEBUG
assert(source);
#if BUILD_VERSION < VERSION_J
#line 74
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this redundant? This #line 74 is placed exactly in line 74, or does it apply to the very next statement?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup #line sets the line number of the next line, not the current one.

Comment on lines +48 to +52
if (fxmix < 0) { // Not possible
update->data.i = -fxmix;
} else {
update->data.i = fxmix;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe an ABS macro?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is how it was in the original source, changing it to a ternary for an ABS macro breaks matching on libgultra_d.

Comment on lines +49 to +51
if (fxmix < 0) { // Not possible
fxmix = -fxmix;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ditto on it breaking matching.

Copy link
Collaborator

@AngheloAlf AngheloAlf left a comment

Choose a reason for hiding this comment

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

LGTM

@Mr-Wiseguy Mr-Wiseguy merged commit 2ab125a into main Sep 3, 2023
30 checks passed
@hensldm hensldm deleted the versions_I_J branch February 27, 2024 00:01
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