Setting Modern as default

[AsparagusEduardo]

Description

@gruxor is happy.

make now does modern, while make agbcc compiles using agbcc. make modern still does its thing.

Discord contact info

AsparagusEduardo

[DizzyEggg]

The future is now

[SBird1337]

The CI seems to still build tests with agbcc. (But also they are currently broken on modern)

[mrgriffin]

The CI seems to still build tests with agbcc. (But also they are currently broken on modern)

Oh yeah... idk how others feel, but I'd really want the tests to run and pass before we merge this PR.

There was a tiny amount of discussion on Discord, but nothing particularly helpful. I haven't had the time to look into this, but a reasonable first step might be to look at the failing tests running in a visual mgba.

[AsparagusEduardo]

Oh yeah... idk how others feel, but I'd really want the tests to run and pass before we merge this PR.

I completely agree.

[AsparagusEduardo]

Set as draft until tests work on modern.

[AsparagusEduardo]

CI set to Modern and tests pass :)

[mrgriffin]

Time to merge this? 👀

[SBird1337]

Can we rebase to upcoming one more time to get CI confirmation? :)

[mrgriffin]

EDIT: SBird already PRed this to upcoming, so just merging upcoming into this PR should be fine.

Can we rebase to upcoming one more time to get CI confirmation? :)

Good call, it's complaining about my ModifyPersonalityForNature test :kekw:

diff --git a/test/battle/trainer_control.c b/test/battle/trainer_control.c
index 99bea0e8cc..192b12980c 100644
--- a/test/battle/trainer_control.c
+++ b/test/battle/trainer_control.c
@@ -123,7 +123,7 @@ TEST("CreateNPCTrainerPartyForTrainer generates different personalities for diff
 
 TEST("ModifyPersonalityForNature can set any nature")
 {
-    u32 personality, nature, j, k;
+    u32 personality = 0, nature = 0, j, k;
     for (j = 0; j < 64; j++)
     {
         for (k = 0; k < NUM_NATURES; k++)

[Jaizu]

I think this PR is broken and needs this 2 aditional changes... right? If someone more savy than me with makefiles can confirm I would be grateful.

[mrgriffin]

I think this PR is broken and needs this 2 aditional changes... right? If someone more savy than me with makefiles can confirm I would be grateful.

Yeah that sounds right. .PHONY is needed because otherwise touch agbcc will mean make agbcc does nothing (until one of its dependencies changes). And building the tools is obviously necessary.

[AsparagusEduardo]

Yeah that sounds right. .PHONY is needed because otherwise touch agbcc will mean make agbcc does nothing (until one of its dependencies changes). And building the tools is obviously necessary.

I'm out until Friday, so someone can PR to this PR the merge with upcoming plus these changes and I'll accept it.

Needs Phony

[mrgriffin]

I'm out until Friday, so someone can PR to this PR the merge with upcoming plus these changes and I'll accept it.

AsparagusEduardo#24

[AsparagusEduardo]

https://www.youtube.com/watch?v=jeMAHzsafcw