Skip to content

Commit

Permalink
Fixed rare crashes on SDL joystick connections
Browse files Browse the repository at this point in the history
  • Loading branch information
efroemling committed Sep 30, 2024
1 parent 1e7a17c commit fdd9d04
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 437 deletions.
62 changes: 31 additions & 31 deletions .efrocachemap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
### 1.7.37 (build 22018, api 9, 2024-09-28)
### 1.7.37 (build 22019, api 9, 2024-09-30)
- Bumping api version to 9. As you'll see below, there's some UI changes that
will require a bit of work for any UI mods to adapt to. If your mods don't
touch UI stuff at all you can simply bump your api version and call it a day.
Expand Down Expand Up @@ -108,6 +108,8 @@
the co-op screen and pressed the trophy toolbar icon to see your league rank
and then pressed back, you would be taken back to the top level main menu. Now
it will take you back to the co-op screen.
- (build 22018) Hardened SDL joystick handling code so the app won't crash if
SDL_JoystickOpen() returns nullptr for whatever reason.

### 1.7.36 (build 21944, api 8, 2024-07-26)
- Wired up Tokens, BombSquad's new purchasable currency. The first thing these
Expand Down
2 changes: 1 addition & 1 deletion src/assets/ba_data/python/baenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@

# Build number and version of the ballistica binary we expect to be
# using.
TARGET_BALLISTICA_BUILD = 22018
TARGET_BALLISTICA_BUILD = 22019
TARGET_BALLISTICA_VERSION = '1.7.37'


Expand Down
25 changes: 17 additions & 8 deletions src/ballistica/base/app_adapter/app_adapter_sdl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "ballistica/base/app_adapter/app_adapter_sdl.h"

#include "ballistica/base/base.h"
#include "ballistica/base/dynamics/bg/bg_dynamics.h"
#include "ballistica/base/graphics/gl/gl_sys.h"
#include "ballistica/base/graphics/gl/renderer_gl.h"
#include "ballistica/base/graphics/graphics.h"
Expand Down Expand Up @@ -555,8 +554,18 @@ void AppAdapterSDL::OnSDLJoystickAdded_(int device_index) {

// Create the joystick here in the main thread and then pass it over to
// the logic thread to be added to the action.
auto* j = Object::NewDeferred<JoystickInput>(device_index);
int instance_id = SDL_JoystickInstanceID(j->sdl_joystick());
JoystickInput* j{};
try {
j = Object::NewDeferred<JoystickInput>(device_index);
} catch (const std::exception& exc) {
Log(LogLevel::kError,
std::string("Error creating JoystickInput for SDL device-index "
+ std::to_string(device_index) + ": ")
+ exc.what());
return;
}
assert(j != nullptr);
auto instance_id = SDL_JoystickInstanceID(j->sdl_joystick());
AddSDLInputDevice_(j, instance_id);
}

Expand Down Expand Up @@ -592,7 +601,7 @@ void AppAdapterSDL::RemoveSDLInputDevice_(int index) {
"GetSDLJoystickInput_() returned nullptr on RemoveSDLInputDevice_();"
" joysticks size is "
+ std::to_string(sdl_joysticks_.size()) + "; index is "
+ std::to_string(index));
+ std::to_string(index) + ".");
return;
}

Expand All @@ -602,7 +611,7 @@ void AppAdapterSDL::RemoveSDLInputDevice_(int index) {
} else {
Log(LogLevel::kError, "Invalid index on RemoveSDLInputDevice: size is "
+ std::to_string(sdl_joysticks_.size())
+ "; index is " + std::to_string(index));
+ "; index is " + std::to_string(index) + ".");
}
g_base->input->PushRemoveInputDeviceCall(j, true);
}
Expand Down Expand Up @@ -636,9 +645,9 @@ auto AppAdapterSDL::GetSDLJoystickInput_(const SDL_Event* e) const
auto AppAdapterSDL::GetSDLJoystickInput_(int sdl_joystick_id) const
-> JoystickInput* {
assert(g_core->InMainThread());
for (auto sdl_joystick : sdl_joysticks_) {
if ((sdl_joystick != nullptr) && (*sdl_joystick).sdl_joystick_id() >= 0
&& (*sdl_joystick).sdl_joystick_id() == sdl_joystick_id)
for (auto* sdl_joystick : sdl_joysticks_) {
if ((sdl_joystick != nullptr) && sdl_joystick->sdl_joystick_id() >= 0
&& sdl_joystick->sdl_joystick_id() == sdl_joystick_id)
return sdl_joystick;
}
return nullptr; // Epic fail.
Expand Down
9 changes: 8 additions & 1 deletion src/ballistica/base/input/device/input_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,14 @@ auto InputDevice::GetPersistentIdentifier() const -> std::string {
return buffer;
}

InputDevice::~InputDevice() { assert(g_base->InLogicThread()); }
InputDevice::~InputDevice() {
// Once we've been added in the logic thread and given an index we
// should only be going down in the logic thread. If our constructor
// throws an exception its possible and valid to go down elsewhere.
if (index_ != -1) {
assert(g_base->InLogicThread());
}
}

// Called to let the current host/client-session know that we'd like to
// control something please.
Expand Down
9 changes: 8 additions & 1 deletion src/ballistica/base/input/device/joystick_input.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "ballistica/base/ui/ui.h"
#include "ballistica/core/core.h"
#include "ballistica/shared/foundation/event_loop.h"
#include "ballistica/shared/foundation/macros.h"
#include "ballistica/shared/python/python.h"
#include "ballistica/shared/python/python_command.h"

Expand Down Expand Up @@ -54,7 +55,13 @@ JoystickInput::JoystickInput(int sdl_joystick_id,
assert(g_core->InMainThread());

sdl_joystick_ = SDL_JoystickOpen(sdl_joystick_id);
assert(sdl_joystick_);
if (sdl_joystick_ == nullptr) {
auto* err = SDL_GetError();
if (!err) {
err = "Unknown SDL error.";
}
throw Exception(std::string("Error in SDL_JoystickOpen: ") + err + ".");
}

// In SDL2 we're passed a device-id but that's only used to open the
// joystick; events and most everything else use an instance ID, so we store
Expand Down
2 changes: 1 addition & 1 deletion src/ballistica/shared/ballistica.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ auto main(int argc, char** argv) -> int {
namespace ballistica {

// These are set automatically via script; don't modify them here.
const int kEngineBuildNumber = 22018;
const int kEngineBuildNumber = 22019;
const char* kEngineVersion = "1.7.37";
const int kEngineApiVersion = 9;

Expand Down
9 changes: 9 additions & 0 deletions src/ballistica/shared/foundation/exception.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ Exception::~Exception() { delete stack_trace_; }

auto Exception::what() const noexcept -> const char* {
// Return a nice pretty stack trace and other relevant info.

// Note: Design-wise it is a bit odd to have what() always return a stack
// trace. It would seem more reasonable and closer to how Python itself
// behaves to have what() simply give the exception message and have a
// separate method to extract the stack trace. However, in cases such as
// crash reports, what() often makes it into the reports and including the
// stack trace there is often useful, so we do things a bit backward;
// including the trace by default and having a separate method to get the
// message without it.
try {
// This call is const so we're technically not supposed to modify ourself,
// but a one-time flattening of our description into an internal buffer
Expand Down
3 changes: 3 additions & 0 deletions src/ballistica/shared/foundation/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,9 @@ enum class SpecialChar : uint8_t {
kLast // Sentinel
};

// NOTE: When adding exception types here, add a corresponding
// handler in Python::SetPythonException.

/// Python exception types we can raise from our own exceptions.
enum class PyExcType : uint8_t {
kRuntime,
Expand Down
Loading

0 comments on commit fdd9d04

Please sign in to comment.