WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Conversation

@jendrikw
Copy link

Description

Configure clangd to allow editing with VSCode. To facilitate this, some declarations are moved around, some types are adjusted and some include guards added.

Compiles to the same rom.

Discord contact info

jendr1k_

src/util.c Outdated
Comment on lines 127 to 132
void StoreWordInTwoHalfwords(u16 *h, u32 w) {
h[0] = (u16)(w);
h[1] = (u16)(w >> 16);
}

void LoadWordFromTwoHalfwords(u16 *h, u32 *w)
{
*w = h[0] | (s16)h[1] << 16;
}
void LoadWordFromTwoHalfwords(u16 *h, u32 *w) { *w = h[0] | (s16)h[1] << 16; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I haven't checked everything, but did you intend to reformat code like in this example?

Copy link
Author

Choose a reason for hiding this comment

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

This reformatting (and others) are on accident, VSCode just really wants to format stuff. I will revert the formatting changes.

else
#endif
lvl = 0;
lvl = 0; // NOLINT(readability-misleading-indentation)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just my opinion, but I kinda hate putting these sorts of "tool-only" comments in the code.

If you're committed to clangd not throwing a spurious warning here, is there a different way the code could be formatted which would read naturally and not throw the warning? Or maybe clangd could be told to assume BUGFIX is defined?

Copy link
Author

Choose a reason for hiding this comment

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

Indenting the line with four spaces is fine for clang, if that's ok for you. I think clangd should have no warnings no matter the setting of BUGFIX.

src/pokedex.c Outdated
u8 menuItem;
u16 *cursorPos;
u16 *scrollOffset;
s16 *scrollOffset; // should also
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this comment is incomplete?

Copy link
Author

Choose a reason for hiding this comment

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

I'll remove it

src/m4a_tables.c Outdated
Comment on lines 246 to 248
#define MOD 0xc4
#define MOD 0xc4
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you meant to commit this change?

Copy link
Author

Choose a reason for hiding this comment

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

will revert

src/list_menu.c Outdated
Comment on lines 1264 to 1265
subsprites[id].x = (s8) 136;
subsprites[id].y = (s8) 136;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We format casts as (s8)136 rather than (s8) 136. There's a few other instances in this file, and I think there's a bunch in other files too (usually to do with unsigned vs signed confusion on GF's part).

Comment on lines +1 to +2
#ifndef REGION_MAP_CITY_TILEMAPS_H
#define REGION_MAP_CITY_TILEMAPS_H
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of these include guards? The src/data/*.h files would only be #included directly into a single .c file (otherwise you'd get multiple definition errors from the linker), so the guards are misleading imo—they state that it's expected to include this file from multiple places.

Comment on lines +1 to +2
#include <stddef.h>

Copy link
Collaborator

Choose a reason for hiding this comment

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

On a similar note, I get that the idea here is to tell your tools that NULL is in scope, but it seems really sketchy to have #include blocks at the top of files that are intended to be included part-way through a .c file.

As a user, I think it would be very confusing if there's a #include in a src/data/*.h file that isn't a no-op—in that situation you would have names and types that only start existing half-way through your .c file, in a way that would be very surprising.

Obviously none of your #includes function like that (or at least I hope they don't!), but I think it's laying a small trap for future users that aren't as savvy about the conventions in pokeemerald.

Comment on lines 4455 to 4462
#ifdef CLANGD
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconstant-conversion"
#endif
sprite->data[4] = (sprite->data[6] = GetBattlerSide(gBattleAnimAttacker)) ? 0x300 : 0xFFFFFD00;
#ifdef CLANGD
#pragma clang diagnostic pop
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there some other way this could be written, or is the problem that 0xFFFFFD00 needs to be written literally like that and can't be -0x300 (or failing that, 0xFD00)?

It's a shame to turn one line into eight 😅

include/main.h Outdated
#ifndef GUARD_MAIN_H
#define GUARD_MAIN_H

#include <stdint.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's stdint.h for?

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.

2 participants