Add standalone GameRunningModal #868
Conversation
| isSteamGame = this.activeGame.activePlatform.storePlatform === StorePlatform.STEAM; | ||
|
|
||
| close() { | ||
| this.$store.commit('closeModals'); |
There was a problem hiding this comment.
Nit but can we rename this event to closeGameRunningModal
Feels like we shouldn't be closing all in one go and instead should close whichever has the priority overlay. (Thinking errors vs game running modals sharing the same event)
There was a problem hiding this comment.
I agree, on the hindsight closing all modals at once doesn't sound that appealing. Renamed the commit name.
|
|
||
| @Component | ||
| export default class GameRunningModal extends Vue { | ||
| activeGame = GameManager.activeGame; |
There was a problem hiding this comment.
Does this change whenever the game is changed via settings?
As far as I'm aware it didn't and that's why binds are in created hooks instead, could be wrong though.
There was a problem hiding this comment.
It does change, since the modal gets recreated/remounted every time the user travels to GameSelectionScreen and returns to Manager screen, and when it's created the value of activeGame has already been updated by other components. If we ever change it so the game can be reselected directly from the Manager screen, we probably need to rethink this as well.
There was a problem hiding this comment.
We could also decouple the component entirely from the GameManager and simply pass in the active game name as a prop to the component. I'd feel most comfortable with that solution as there's less coupling that way.
src/store/ModalsModule.ts
Outdated
| isGameRunningModalOpen: boolean; | ||
| } | ||
|
|
||
| export default { |
There was a problem hiding this comment.
Potentially nicer to be under `/src/store/modules/ModalsModule.ts
There was a problem hiding this comment.
Moved to subdir.
anttimaki
force-pushed
the
standalone-game-running-modal
branch
from
eb6eb95
to
e03cfaa
Compare
|
Btw should something be done about the Codacy code analysis failing? It says
Would this issue get automatically resolved once the earlier PR in the chain is merged and this points to |
| await launch(this.activeGame, this.contextProfile!, mode); | ||
| } catch (error) { | ||
| if (error instanceof R2Error) { | ||
| this.gameRunning = false; | ||
| this.$store.commit("closeModals"); |
There was a problem hiding this comment.
This should probably be calling closeGameRunningModal now that it was renamed
|
|
||
| @Component | ||
| export default class GameRunningModal extends Vue { | ||
| activeGame = GameManager.activeGame; |
There was a problem hiding this comment.
We could also decouple the component entirely from the GameManager and simply pass in the active game name as a prop to the component. I'd feel most comfortable with that solution as there's less coupling that way.
The modal was originally built in the navigation menu since that's where the games are launched from, but decoupling these two will hopefully make it easier to customize the NavigationMenu in different views and mod manager variants. Since the modal and the NavigationMenu are no longer a direct parent- child relation, the status of modal visibity is stored in Vuex store. A separate store module was created for this purpose - the idea is that in the future it could be used to manage more modals than just the one it currently does.
anttimaki
force-pushed
the
standalone-game-running-modal
branch
from
e03cfaa
to
4127b9b
Compare
The modal was originally built in the navigation menu since that's where the games are launched from, but decoupling these two will hopefully make it easier to customize the NavigationMenu in different views and mod manager variants.
Since the modal and the NavigationMenu are no longer a direct parent- child relation, the status of modal visibity is stored in Vuex store. A separate store module was created for this purpose - the idea is that in the future it could be used to manage more modals than just the one it currently does.