Skip to content

Commit

Permalink
Updated the comments in the code for better clarity and accuracy.
Browse files Browse the repository at this point in the history
  • Loading branch information
dnasdw committed Jun 10, 2024
1 parent d679902 commit 1a7cd7e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
14 changes: 13 additions & 1 deletion src/bmarch.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,19 @@

#include "bmarch.h"


// To avoid compilation errors with modern gcc compilers due to forced function type conversion during external calls,
// the function implementing the same logic was divided into two versions with different return types.
// Theoretically, this should be a bug in the official code, and there should only be one version of the function with a return type of s8:
// 1. In the header file (.h), the function is declared with a return type of s32, while in the source file (.c), the function is defined with a return type of s8.
// 2. In such a case, if the current .c file does not include the corresponding .h file, it can be compiled successfully with the agbcc compiler,
// but this affects the compilation dependency relations in the Makefile.
// If we keep only one version of the function with a return type of s8,
// while declaring a version with a return type of s32 before external calls,
// it can also be compiled successfully with agbcc.
// However, since all function implementations are fully inlined in the current compilation unit,
// the resulting assembly file (.s) will not contain the symbol for this function, causing it to disappear eventually.
// Adding the extern keyword before the function definition can force modern gcc compilers to retain the function,
// but this leads to the issue of the function eventually disappearing when compiled with agbcc.
inline s32 IsBallista(struct Trap* trap) {

if (!trap) {
Expand Down
6 changes: 4 additions & 2 deletions src/bmmenu.c
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,10 @@ void ItemSubMenuEnd(struct MenuProc* menu) {
return;
}

// In theory, this function should have a second parameter, but it has never been used in the code.
// If we keep this second parameter, calling the function in the code below after type conversion will result in compilation errors with modern gcc compilers.
// If we remove the second parameter, it will not cause any issues even if a second parameter (i.e., the r1 register) is passed in other non-explicit calls.
// This issue is likely due to a lack of synchronization in the official code.
u8 MenuCommand_SelectNo(struct MenuProc* menu/*, struct MenuItemProc* menuItem*/) {
SetTextFont(NULL);

Expand All @@ -1024,7 +1028,6 @@ u8 sub_8023550(struct MenuProc* menu) {
ProcPtr proc;

sub_8023538(menu);
// This is really caused by implicit declaration
MenuCommand_SelectNo(menu);

proc = StartOrphanMenu(&gItemSelectMenuDef);
Expand Down Expand Up @@ -2279,7 +2282,6 @@ u8 AttackBallistaCommandUsability(const struct MenuItemDef* def, int number) {

trap = GetTrapAt(gActiveUnit->xPos, gActiveUnit->yPos);

// This is really caused by implicit declaration
isBallista = IsBallista(trap);

if (isBallista == 0) {
Expand Down

0 comments on commit 1a7cd7e

Please sign in to comment.