[Refactor] Create global scene variable

[DayKev]

What are the changes the user will see?

N/A

Why am I making these changes?

Currently to access the active scene you need to awkwardly pass it around thorough various functions/etc, and if it's not already being passed to the context you're working in you have to refactor the function/etc to add a scene passaround parameter/whatever and then potentially update every instance of it throughout the code.

What are the changes from a developer perspective?

All the random scene function params, class fields, etc are obliterated removed and replaced with a single global scene variable.

How to test the changes?

Play the game, run the tests.

Checklist

• I'm using beta as my base branch
• There is no overlap with another PR?
• The PR is self-contained and cannot be split into smaller PRs?
• Have I provided a clear explanation of the changes?
• [ ] Have I considered writing automated tests for the issue?
• [ ] If I have text, did I make it translatable and add a key in the English locale file(s)?
• Have I tested the changes (manually)?
  • Are all unit tests still passing? (npm run test)
• [ ] Are the changes visual?
  • [ ] Have I provided screenshots/videos of the changes?

[MokaStitcher]

Found the sound issue I think (see comment in evolution phase)
Haven't checked anything else yet. Did the ME tests pass because you skipped the ones that were failing for now?

[torranx]

Can you please update the "Why" section on the ff:

• why can you say its easier development?
• what does this solve?
• why is this the approach we made (ie global variable vs using something from phaser's SceneManager (I'm not too knowledgeable on phaser but I've seen some scene getters on docs)?

I agree that it's🍝 to slap the BattleScene on everything but I just wanna understand this change doesn't turn out to be the same.

[DayKev]

I sure love how impossible it is to batch apply suggestions to a large PR, thanks GitHub.

[DayKev]

Can you please update the "Why" section on the ff:

Yeah, I just wanted to get the PR up before sleeping so I could get some feedback (esp on the sound issue).

• why can you say its easier development?
• what does this solve?

Currently to access the scene you need to awkwardly pass it around thorough various functions/etc, and if it's not already being passed to the context you're working in you have to refactor the function/etc to add a scene passaround parameter/whatever and then potentially update every instance of it throughout the code.

• why is this the approach we made (ie global variable vs using something from phaser's SceneManager (I'm not too knowledgeable on phaser but I've seen some scene getters on docs)?

I agree that it's🍝 to slap the BattleScene on everything but I just wanna understand this change doesn't turn out to be the same.

I'm not too familiar with Phaser's tool(s) that we have access to for this, and their documentation permalinks are broken when using Google as I've just realized when trying to look it up. If you know of a better way that'd be great. I went with what seemed like the simplest method.

[DayKev]

Found the sound issue I think (see comment in evolution phase) Haven't checked anything else yet. Did the ME tests pass because you skipped the ones that were failing for now?

Yeah I disabled them (for now) and marked them with // TODO: .... There was an issue that broke a bunch of tests (since a lot of ME tests are very similar), plus a strange intermittent(?) issue with the SelectModifierPhase tests and an issue I don't understand in the "bug superfan" ME test.

General ME tests issue: 55d1b14#diff-b25c954eb59b69c9bdf7245abdfcb9428f37a4d2b7c68bcf2177aab1810889d8R129

SMP issue: 55d1b14#diff-6dcfc99d6f0b54ca275b4778e9f553ec1ae6cbee7698442de4731ef524409df2R16

"bug superfan" ME issue: 55d1b14#diff-dfe138746b004afe72d3f75d913fc5a5c84a9d52879f12a11afb1200c4efd3d9R386

I tested the ME issues in-game and everything worked fine, so it's a test-specific issue, I'm just not sure what's going on.

[MokaStitcher]

Stopping for now, but this should fix the bug superfan encounter issue at least (explanation in second comment bc I keep forgetting github doesn't organize them by line)

[DayKev]

New issue discovered: async bullshit!

broken stuff

broken.i18n.mp4

not all the stuff is broken though!

Some of the i18n just randomly doesn't load at the right time.

[PigeonBar]

Take this with several grains of salt, but:

1. The async malfunctioning seems to be caused by stuff being imported too early. Everything importing globalScene seems to cause battle-scene.ts to also need to import from settings.ts in a way that initializes its various objects before i18next is ready.
2. The settings locales work correctly after I moved globalScene to a new file global-scene.ts using VSCode's refactor tools:

import BattleScene from "#app/battle-scene";

// BattleScene.name;

export let globalScene: BattleScene;

export function initGlobalScene(scene: BattleScene): void {
  globalScene = scene;
}

3. This solution seems to solve the locales issue by delaying the imports in battle-scene.ts until they are truly needed. As further evidence, uncommenting the commented line causes locales to break again.

[DayKev]

Huh, I tried the opposite (making sure battle-scene.ts didn't import settings.ts) but I guess that could do it. I wonder if that'll fix anything else...

[DayKev]

Okay I think I found the issue with the SelectModifierPhase tests, the way they're mocking the set up is incorrect in some way. Using the standard test setup (game.classicMode.startBattle([ Species.ABRA, Species.VOLCARONA ]); etc) instead of whatever they're doing to manually run a SMPhase fixed the tests. Though two of them I couldn't get to work yet because making the UI do things in tests is hell...

[DayKev]

Well whatever, the two SMPhase tests I couldn't fix weren't actually testing the thing they were supposed to be testing anyway.

That just leaves the ME tests that were breaking upon reaching MysteryEncounterRewardsPhase...

[DayKev]

Everything is fixed!
🎉

[frutescens]

approving to bite the merge conflict bullet

[DayKev]

If only GitHub wasn't dumb and could fix these fake merge conflicts automatically instead of needing me to do it locally.

[frutescens]

This is needed for cleaning up RNG too.