Don't spawn a player immediately when a joystick is connected#3537
Don't spawn a player immediately when a joystick is connected#3537Semphriss wants to merge 1 commit intoSuperTux:masterfrom
Conversation
I personally think you should fade in a popup at the bottom of the screen. "Press A to join as a new player". Of course, only do that for new controllers. You could use a |
| { | ||
| auto it = m_game_controllers.find(SDL_GameControllerFromInstanceID(ev.which)); | ||
| auto* controller = SDL_GameControllerFromInstanceID(ev.which); | ||
| auto it = m_game_controllers.find(controller); | ||
|
|
||
| if (it == m_game_controllers.end() || it->second < 0) | ||
| if (it == m_game_controllers.end()) | ||
| return; | ||
|
|
||
| if (it->second < 0) | ||
| { | ||
| if (ev.button == SDL_CONTROLLER_BUTTON_A && g_config->multiplayer_auto_manage_players) | ||
| { | ||
| autobind_controller(controller); | ||
| } | ||
| else | ||
| { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| player_id = it->second; | ||
| } |
There was a problem hiding this comment.
I don't like that this code is done in it's own scope like this (this makes the Controller& controller bit later seen misleading). Maybe it should just passed to another function.
There was a problem hiding this comment.
The scope was already there before this PR. How would you like me to organize it?
There was a problem hiding this comment.
Move it to a function if possible
There was a problem hiding this comment.
I'm not sure which block could be moved to a new function, as there is a lot of code that would be needed for both functions. The nested return must interrupt both functions, and fetching the controller need to be done to obtain the player ID. If I were to make it into a separate function, there would be a lot of duplicated code, or a tight coupling of the two functions which may take away the purpose of making a new function.
Alternatively, I can rename the controller variable to something like sdl_controller and remove the block scope.
Semphriss
force-pushed
the
defer-spawn
branch
from
8cf5bdd to
3920481
Compare
|
Don't merge/review, I mistakenly committed stuff I shouldn't have. Edit: This is fixed now. |
Semphriss
force-pushed
the
defer-spawn
branch
from
3920481 to
18ef2eb
Compare
|
Request for review when you're ready! |
|
@swagtoy I haven't yet addressed the two discussions that are still open since I would need clarification before continuing, not sure if they still apply |
|
Ope... mised that... lot on my plate at the moment, ill return soon |
Semphriss
force-pushed
the
defer-spawn
branch
from
18ef2eb to
ae5857d
Compare
Semphriss
force-pushed
the
defer-spawn
branch
from
ae5857d to
705583a
Compare
Semphriss
force-pushed
the
defer-spawn
branch
3 times, most recently
from
eb04223 to
8557172
Compare
A connected joystick will no longer automatically spawn a player until the user presses a button.
Semphriss
force-pushed
the
defer-spawn
branch
from
8557172 to
16d6d6f
Compare
swagtoy
force-pushed
the
master
branch
2 times, most recently
from
806ca6a to
c12b71b
Compare
3 participants
A connected joystick will no longer automatically spawn a player until the user presses A (Game controllers) or Jump (Joysticks).
The button is not reconfigurable in-game, but I can change it as needed.
Design
If a controller is plugged in, no player is created until that user presses A. If a controller is disconnected, the player's character is removed from the game, but its information remains in memory, and when the controller is plugged back in, the player respawns immediately. This is to avoid too much frustration in case a game controller is unplugged by accident.
I tried to follow the code standards based on the surrounding code and my common sense, but I may have missed something. Do let me know if something needs to be modified code-wise.
To be discussed: How should we notify the user that a controller was plugged in and that they can press A to join immediately?