Kitteh6660 / Corruption-of-Champions-Mod

CoC source from fenoxo, modded by Kitteh6660
232 stars 97 forks source link

[Bug] local ant build breaks CoC_Main.sol #1371

Closed Stadler76 closed 5 years ago

Stadler76 commented 5 years ago

I have checked the following before submitting this issue:

Overview

It seems, that running the ant tests locally breaks CoC_Main.sol. Probably by creating an empty one once the testing starts.

Game version

Latest dev build. Probably broken due to recent (don't ask me how recent) changes to the test suite.

Expected behaviour

CoC_Main.sol is unchanged.

Actual behaviour

CoC_Main.sol get recreated.

How often can this be reproduced?

Every time you run ant locally

Steps to reproduce the issue

  1. Play the game locally, get some achievements. Unlock stuff ... whatever changes the CoC_Main.sol though 'normal' gameplay.
  2. Run ant.
Stadler76 commented 5 years ago

Ok, I hunted it down: It happens somewhere between commit 7f589c92779f378ed7751992176a5ac919db824e (can reproduce) and commit bbe548dbe2678bafbff27a3ca68afc2656ef7298 (can not reproduce). I assume, its one of the tests, that is to be blamed, but I'm not in the mood to do more ~10 minute ant runs right now.

@brrritssocold: Maybe you have an idea?

Stadler76 commented 5 years ago

Nailed it: Commit 94a8f096e11505a6b0150046800ec11ba631a5d0 is the one to blame. To be precise: Its the KangaFruitTest. Kanga Fruit may trigger a game over which grants the game over achievement thus triggering a savePermObject-call:

public static const GENERAL_GAME_OVER:int = 112; //Get a Bad End.

I wonder, if there's a way to check in savePermObject whether we're inside a test or not. Well anyway: skipping savePermObject when running tests should be the best way to avoid accidentally overwriting it during tests.

Stadler76 commented 5 years ago

@brrritssocold: Here a potential 'fix' for this. I haven't changed the other unit tests, since they're not affected and I wanted to minify the diff and since this is 'one' way to do it and maybe not the best way.

diff --git a/classes/classes/CoC.as b/classes/classes/CoC.as
index 94c424514..7d3b983f5 100644
--- a/classes/classes/CoC.as
+++ b/classes/classes/CoC.as
@@ -335,6 +335,7 @@ package classes
        // Declare the various global variables as class variables.
        // Note that they're set up in the constructor, not here.
        public var debug:Boolean;
+       public var unitTest:Boolean;
        public var ver:String;
        public var version:String;
        public var versionID:uint = 0;
@@ -499,9 +500,10 @@ package classes
         *
         * @param injectedStage if not null, it will be used instead of this.stage
         */
-       public function CoC(injectedStage:Stage = null)
+       public function CoC(injectedStage:Stage = null, unitTest:Boolean = false)
        {
            var stageToUse:Stage;
+           this.unitTest = unitTest;

            if (injectedStage != null)
            {
diff --git a/classes/classes/Saves.as b/classes/classes/Saves.as
index 90f965d99..f317efdaa 100644
--- a/classes/classes/Saves.as
+++ b/classes/classes/Saves.as
@@ -590,6 +590,9 @@ public function loadGame(slot:String):void

 //Used for tracking achievements.
 public function savePermObject(isFile:Boolean):void {
+   if (getGame().unitTest) {
+       return;
+   }
    //Initialize the save file
    var saveFile:*;
    var backup:SharedObject;
diff --git a/test/classes/Items/Consumables/KangaFruitTest.as b/test/classes/Items/Consumables/KangaFruitTest.as
index 1be0768b9..d2a0027ff 100644
--- a/test/classes/Items/Consumables/KangaFruitTest.as
+++ b/test/classes/Items/Consumables/KangaFruitTest.as
@@ -31,7 +31,7 @@ package classes.Items.Consumables
        [BeforeClass]
        public static function setUpClass():void
        {
-           kGAMECLASS = new CoC(StageLocator.stage);
+           kGAMECLASS = new CoC(StageLocator.stage, true);
        }

        [Before]
brrritssocold commented 5 years ago

Sorry for the late response, been busy. Your suggestion is on the right track, will create a PR with a fix ASAP.

FYI: If you have a script or program that returns zero / non-zero you can run it over a set of commits with git bisect run {command}. See git bisect run docs

Obviously in this case you could not get around running the whole suite, as the issue was caused by a test, but it would have allowed you to automate the search for the commit.