From b71e0fc1308a4c0ac92e839ab86133db090ad279 Mon Sep 17 00:00:00 2001
From: wgroeneveld <wouter.groeneveld@gmail.com>
Date: Sat, 15 Dec 2018 19:34:47 +0100
Subject: [PATCH] major performance gain by optimizing GBAEngine::update()

---
 demos/demo3-foodthrowing/src/food_scene.cpp   | 19 +++++++++++++++++++
 .../sprites/sprite_manager.h                  |  1 +
 engine/src/gba_engine.cpp                     | 13 ++++++++++---
 engine/src/sprites/sprite_manager.cpp         | 17 +++++++++++------
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/demos/demo3-foodthrowing/src/food_scene.cpp b/demos/demo3-foodthrowing/src/food_scene.cpp
index a1b1b13..c785547 100644
--- a/demos/demo3-foodthrowing/src/food_scene.cpp
+++ b/demos/demo3-foodthrowing/src/food_scene.cpp
@@ -48,6 +48,7 @@ VECTOR FoodScene::rotateAround(VECTOR center, VECTOR point) {
 void FoodScene::tick(u16 keys) {
     avatar->animateToFrame(0);
     bool allowedToShoot = false;
+    int oldBulletSize = bullets.size();
 
     if(bulletCooldown > 0) {
         bulletCooldown--;
@@ -82,6 +83,9 @@ void FoodScene::tick(u16 keys) {
     }
 
     avatar->rotate(avatarRotation);
+    if(oldBulletSize != bullets.size()) {
+        engine.get()->updateSpritesInScene();
+    }
 
     for(auto &b : bullets) {
         b->tick();
@@ -118,4 +122,19 @@ void FoodScene::load() {
 
     // rotation of a point on a circle within the resolution means our radius should be big enough
     defaultBulletTarget = { GBA_SCREEN_WIDTH / 2, GBA_SCREEN_HEIGHT + (GBA_SCREEN_WIDTH / 2) - avatar->getCenter().y + 40};
+
+
+    /*
+    for(int i = 0; i < 10; i++) {
+        for(int j = 0; j < 4; j++) {
+            bullets.push_back(createBullet());
+
+            auto &b = bullets.at(bullets.size() - 1);
+            b->getSprite()->moveTo(10 + (i * 20), 10 + (j * 20));
+            if(j >= 1) {
+                b->getSprite()->moveTo(10 + (i * 20), 100 + (j * 20));
+            }
+        }
+    }
+     */
 }
\ No newline at end of file
diff --git a/engine/include/libgba-sprite-engine/sprites/sprite_manager.h b/engine/include/libgba-sprite-engine/sprites/sprite_manager.h
index 880638a..dce1d12 100644
--- a/engine/include/libgba-sprite-engine/sprites/sprite_manager.h
+++ b/engine/include/libgba-sprite-engine/sprites/sprite_manager.h
@@ -21,6 +21,7 @@ private:
 public:
     int getSpriteSize() { return sprites.size(); }
 
+    void hideAll();
     void add(Sprite* sprite);
     void set(std::vector<Sprite*> sprites);
     void persist();                      // copies over image and palette data to VRAM, modifies sprite OAM indiches
diff --git a/engine/src/gba_engine.cpp b/engine/src/gba_engine.cpp
index 5a73d21..826258d 100644
--- a/engine/src/gba_engine.cpp
+++ b/engine/src/gba_engine.cpp
@@ -113,6 +113,8 @@ GBAEngine::GBAEngine() {
 }
 
 void GBAEngine::update() {
+    // main update loop, in while(true) {}.
+    // WARNING - keep amount of instructions as minimal as possible in here!
     if(sceneToTransitionTo) {
         currentEffectForTransition->update();
 
@@ -122,12 +124,16 @@ void GBAEngine::update() {
     }
 
     u16 keys = readKeys();
+    // main scene update loop call. This *might* take a while.
     currentScene->tick(keys);
 
-    if(currentScene->sprites().size() != spriteManager.getSpriteSize()) {
-        updateSpritesInScene();
-    }
+    // Intentionally commented out: asking the scene for sprites() rebuilds the vector each time
+    // Causing a big performance hit. Instead, you should call updateSpritesInScene() yourself!
+    // if(currentScene->sprites().size() != spriteManager.getSpriteSize()) {
+    //     updateSpritesInScene();
+    // }
 
+    // TODO use software interrupt Vsyncing instead of 2 wasteful whiles
     vsync();
     spriteManager.render();
 }
@@ -156,6 +162,7 @@ void GBAEngine::setScene(Scene* scene) {
             TextStream::instance().clear();
         }
     }
+    spriteManager.hideAll();
     scene->load();
 
     auto fgPalette = scene->getForegroundPalette();
diff --git a/engine/src/sprites/sprite_manager.cpp b/engine/src/sprites/sprite_manager.cpp
index 760dce8..3be02bf 100644
--- a/engine/src/sprites/sprite_manager.cpp
+++ b/engine/src/sprites/sprite_manager.cpp
@@ -29,6 +29,7 @@ void SpriteManager::add(Sprite* sprite) {
 }
 
 void SpriteManager::render() {
+    // WARNING - This is called every time in the main update loop; keep amount of instructions as minimal as possible in here!
     if(!initialized) {
         failure_gba(Cant_Render_Before_Init);
     }
@@ -37,7 +38,7 @@ void SpriteManager::render() {
 }
 
 void SpriteManager::persist() {
-   copyOverImageDataToVRAM();
+    copyOverImageDataToVRAM();
     initialized = true;
 }
 
@@ -46,9 +47,6 @@ void SpriteManager::copyOverSpriteOAMToVRAM() {
     int affineIndex = 0;
 
     for(auto sprite : this->sprites) {
-        if(affineIndex > MAX_AFFINE_SIZE) {
-            failure_gba(MaxSpritesWithAffineReached);
-        }
         sprite->update();
 
         auto oam = sprite->oam.get();
@@ -66,8 +64,15 @@ void SpriteManager::copyOverSpriteOAMToVRAM() {
 
         i++;
     }
-    for(int j = i; j < MAX_SPRITE_SIZE; j++) {
-        oam_mem[j].attr0 = ATTR0_HIDE;
+
+    if(affineIndex > MAX_AFFINE_SIZE) {
+        failure_gba(MaxSpritesWithAffineReached);
+    }
+}
+
+void SpriteManager::hideAll() {
+    for(int i = 0; i < MAX_SPRITE_SIZE; i++) {
+        oam_mem[i].attr0 = ATTR0_HIDE;
     }
 }