asrob-uc3m / robotDevastation

A new-generation shooter with augmented reality and real robots. You can play online with other users with your PC, moving robots in championships and campaigns: all 24/7!
http://asrob-uc3m.github.io/workgroups/2017-05-28-robot-devastation.html
GNU Lesser General Public License v2.1
9 stars 4 forks source link

Create ScreenManager #71

Closed David-Estevez closed 7 years ago

David-Estevez commented 7 years ago

SDL requires to pump events in the same thread as the graphical output thread. Therefore, we need to have a ScreenManager to allow us to call the required functions in the required places. The design of the class is as follows:

The proposed interface for the class is the following:

class ScreenManager
{
    public:
        void setCurrentScreen(RdScreen* screen);
        int show();
        int updateProperty(String property, String value); 

         static int initSDL();
         static int cleanSDL();

     private:
         RdScreen * screen;
         Mutex mutex;
}

This will likely fix:

David-Estevez commented 7 years ago

ScreenManager and its SDL implementation, SDLScreenManager, have been added at 8aa7900

This is a basic implementation that does not allow direct manipulation of RdScreen properties from the ScreenManager interface. Basically, there are two things left to implement: SDLWindow is owned by each RdScreen independently (so new windows are created for each RdScreen) and there is no mutex protection on property changes.

David-Estevez commented 7 years ago

ScreenManager now owns the window object (changes done at c155bf7)

The role of the RdScreen is shifted from displaying the screen to just drawing the screen onto the screen surface provided by the game window object.

For that purpose there are some changes in the APIs:

Once this migration is ended, we can start to integrate the ScreenManager in the different States of the game. When all the code is successfully migrated, the show() method can be finally deleted as it is no longer needed. Removing it earlier would mean breaking all the current code.

David-Estevez commented 7 years ago

API migration on RdScreens has been done at 54206b3

Removing the show() method breaks the old game states, as they rely on the old API. So the next step is to update the game states to show the RdScreens through the ScreenManager.

David-Estevez commented 7 years ago

Game states have been updated at a2563302b8

But, when running the tests, the second time it refreshes the SDL window the program crashes with a segmentation fault. This only happens in GameState and DeadState . InitState works perfectly. I will keep debugging this to find the bug.

David-Estevez commented 7 years ago

The source of the error seems to be calling the show() method from different threads in the program. When placing all calls to show() within the corresponding loop() methods the program does not crash (but the screen does not get refresh, so it still does not work).

I'll keep researching....

David-Estevez commented 7 years ago

The source of the bug was that even if the global instance of the ScreenManager owned the SDL window, SDL is designed to work with the window in the thread that created the window. Therefore when I attempted drawing from another thread it yielded a Segmentation Fault.

I considered two possible solutions: controlling (showing) the window from the main thread (outside the states) or deleting the window each time a new RdScreen is set. I selected the second one, implemented at 099b19f

A minor flaw derived from this choice is that the window is repositioned each time it is deleted. But this can be solved with a few variables to remember the old state of the window when destroyed.

David-Estevez commented 7 years ago

ScreenManager integrated with Robot Devastation main program at e6d7fa9

Mutex protection of update() and show() is still not implemented, but everything seems to work. I'll leave the issue open until mutex protection isimplemented. done at 67420528ad54d8737d4cb0f51e815f7273b46601