Moving TFs out of Mutations.as

[Stadler76]

• Finally moved all remaining TFs out of Mutations.as
• In addition to that I've moved the remaining TFs that were scattered around the codebase into the classes.Items.Consumables-package.
• Since there's no more need for a separate MutationsHelper I've merged it with Mutations
• Note: I've turned Mutations into a singleton in PR #329 because that nasty old workaround caused, that there were multiple (two to be precise) instances of Mutations where there should only be one. @brrritssocold: For a similar reason I've designed the Output-class as a singleton. Hence why I've named the getter method init() and not getInstance() or get instance(). For me init() means: Do it once and avoid doing it again. public function getInstance() or public function get instance() implies easy access or let's say: "Use it as often as you want to", which IMHO should be avoided and is not the purpose of a singleton (doesn't count for unit tests though).

PS: I suggest to join the Discord Server to discuss this and other changes to the CoC codebase.

[Stadler76]

Quick and dirty idea of some kind of a wrapper/hook around kGAMECLASS.player = newPlayer;:

public var player:Player;
private var playerStore:Vector.<Player> = new Vector.<Player>();
public function storePlayer(newPlayer:Player):void
{
    playerStore.push(player);
    player = newPlayer;
}
public function restorePlayer():void
{
    if (playerStore.length === 0)
        return;

    player = playerStore.pop();
}
public function rewindPlayer():void
{
    if (playerStore.length === 0)
        return;

    player = playerStore[0];
    playerStore = [];
}

[brrritssocold]

Singletons

The getInstance() method would have been fine, as getInstance() will always return the same instance (the goal of the singleton pattern). The issue is with the static nature that is tied to the singleton pattern.

People will misuse it (i.e. call it inside a method) when instead a instance should be passed as a parameter. The only solution to this is reviewing PRs and request code changes when singleton abuse is discovered.

Player Store

What is the goal / idea behind this? Can you give some context?

Side note: Calling new Player all over the place causes issues with injected instances. e.g. kGAMECLASS.player = new Player now kGAMECLASS.player will point to a different instance than was injected into a instance (see GenderDebug for example)

Was thinking of a clear or reset method that would also be called from the constructor to return the instance to a initial state. Possibly also clone function if a instance needs to be copied.

Corruption-of-Champions-Mod> ag -Q 'new Player' -c --stats .\classes\

-- snip --

77 matches
46 files contained matches
798 files searched
32536514 bytes searched
0.276101 seconds

[Stadler76]

@brrritssocold wrote:

What is the goal / idea behind this? Can you give some context?

Actually I wanted to solve the kGAMECLASS.player = player; in MutationsTest.as but had UrtaQuest.as in mind, where some ugly player-swapping is been done. Anyway: It was a 'quick and dirty' idea, not a definite solution.

For MutationsTest.as I came up with this:

diff --git a/classes/classes/Items/Consumable.as b/classes/classes/Items/Consumable.as
index 1a1cbbef1..4e3c4c47f 100644
--- a/classes/classes/Items/Consumable.as
+++ b/classes/classes/Items/Consumable.as
@@ -26,7 +26,14 @@ package classes.Items

        protected function get output():Output { return kGAMECLASS.output; }
        protected function get credits():Credits { return kGAMECLASS.credits; }
-       protected function get player():Player { return kGAMECLASS.player; }
+       private var _player:Player;
+       protected function get player():Player
+       {
+           if (_player === null) {
+               _player = kGAMECLASS.player;
+           }
+           return _player;
+       }
        protected function get prison():Prison { return kGAMECLASS.prison; }
        protected function get flags():DefaultDict { return kGAMECLASS.flags; }
        protected function get camp():Camp { return kGAMECLASS.camp; }
@@ -37,6 +44,11 @@ package classes.Items
            super(id, shortName, longName, value, description);
        }

+       public function setPlayer(player:Player):void
+       {
+           _player = player;
+       }
+
        override public function get description():String {
            var desc:String = _description;
            //Type
diff --git a/test/classes/Items/MutationsTest.as b/test/classes/Items/MutationsTest.as
index 80aa0a117..9e2487035 100644
--- a/test/classes/Items/MutationsTest.as
+++ b/test/classes/Items/MutationsTest.as
@@ -51,8 +51,7 @@ package classes.Items {
        private function drinkBova(count:int, type:int, player:Player):void {
            var consumable:Consumable = new LaBova(type);

-           // Override global player value because intended(?) Maybe come up with a better solution, like a hook(???) ~Stadler76
-           kGAMECLASS.player = player;
+           consumable.setPlayer(player);

            for (var i:int = 0; i < count; i++) {
                consumable.useItem();

á la: Pass the player-var to where its needed and not to the global kGAMECLASS Its still kind of workaround-ish since I wanted to minimize the impact to the existing codebase but IMHO better, than abusing the global.