search

med-material / Whack_A_Mole_VR

Whack-A-Mole in VR
MIT License
3 stars 15 forks source link

Data Saving System #214

Closed Xav1204 closed 1 year ago

Xav1204 commented 1 year ago

We needed to update UI during a save to show to the therapist that the save is incomplete. So for this, we have a warning message panel that appear when we try to quit the app during the save. Moreover, we have an indicator of the total files that we need to save and how many are saved. When the save is done, we can restart a game. There will maybe be some functions that will disappear if we succeed to implement the multi-save system.

Xav1204 commented 1 year ago

@bastianilso I think that it's good now for the warning panel. I used 3 Unity Events :

  1. The first one is used to manage the spawn of the start game button in order to restart a game after the end of the save. However, as we want to create a multi save system in order to prevent the therapist to wait until the end of the save, this Event should disappear in the future and we will hand over the Reset function in GameStateContainer.cs.
  2. The second one is used to call the function OpenWarningPanel in the TherapistUI script in order to open the panel and inform the therapist that is save is not done and he need to wait until the end of this one. Moreover, we need to disable the exit button because if the therapist click on this button multiple times, the app can crash and the datas will be lost. However, as the app will be closed after the save, it's not necessary to reactivate this button.
  3. The last event is used to close the app if the save is done. It can close the app when the warning panel is opened or closed.

We have also a bool value in our Unity Event in order to manage the opening of the warning panel and the closing of the app if this one is opened.

Finally, to know if the save is done, I look if the last label in the logstore is clear and if it's the case, the save is done.

Please, let me know if it's look right for you now.

bastianilso commented 1 year ago

@Xav1204 this was a lot of comments, but we are also dealing with a data saving system which introduce a lot of new interaction between classes in Whack-A-Mole, that we need to get right. :)

Please respond in-line to the comments i provided above with rationales, suggestions, questions so I follow your train of thought on this, thanks!

Xav1204 commented 1 year ago

@bastianilso I think that it's good now. I simplified the code at the most that I could.

So please tell me if you it looks right for you and don't hesitate to tell me also if you have any questions or doubts.

bastianilso commented 1 year ago

thanks @Xav1204 . I will take a look at this on monday and get back to you. There are still some communication I think we can simplify. The main thing is that TherapistUI should not contain logic about quitting, its merely handling IO between User Interface and managers. In addition, ApplicationManager should subscribe and react to updates from LoggingManager, and behave appropriately based on which saving state LoggingManager is reporting. LoggingManager is only reporting a saving state, whoever listens to this information will decide which parts of the information is relevant to them. LoggingManager does not know about restarting applications, it just announces its own state. Here is a conceptual overview:

LoggingManager: should only report its saving state, e.g. is CSV saving, CSV saving progress, is CSV done, did CSV saving fail..

TherapistUI/GameStateContainer: listens to LoggingManager's updates and updates UI based on state information from LoggingManager saving. (Saving....1/3) etc.

ApplicationManager: Listens to LoggingManager's updates. If the CloseGame() is called while LoggingManager reported CSV is still saving, it prevents the game from quitting and instead asks TherapistUI to show "Saving Data.." Dialog. When LoggingManager informs ApplicationManager that saving is done and a previous call to ClosedGame() has been observed, ApplicationManager can then safely quit the game.

hendrikknoche commented 1 year ago

@bastianilso, should the loggingManager also report on whether it is currently in the state of continuously logging - which will require saving at some point?

bastianilso commented 1 year ago

@hendrikknoche good point. LoggingManager could report a "Logging" state, it would be very logical and expected. It would enable us to e.g. react if users try to exit Whack-A-Mole while a game is running. We could for example then stop the game and show the "Saving data.." dialog and finally quit afterwards.

Xav1204 commented 1 year ago

@bastianilso it's good, all the improvements have been made.

Xav1204 commented 1 year ago

@bastianilso I've made the changes, I hope that it's that you really want especially with the LoggingManager.SaveStatus enum in applicationManager. Moreover, as you've merged a pull request, a conflict with the mainscene.unity appeared and you sent us a mail and explained us how to solve it but I didn't succeed to manage the conflict. If you're at the university tomorrow, could you please come and explain to me and Lucas how to solve this conflict in the future please ?