Skip to content

Commit

Permalink
fix level-loading crashes
Browse files Browse the repository at this point in the history
When new levels are loaded the Hull and Level/Set menus are re-created from scratch.  This would leave dangling pointers in nanogui::Screen (mDragWidget and mFocusPath).  The "best" fix would probably be to replace all Widget pointers with shared_ptr<Widget>.  But this is a quick fix that gets the job done.

I also added "/dbg rload X" which will load a new level every X seconds.  This was useful for recreating this bug but also might be useful for discovering other random level-load issues in the future so I left it in.
  • Loading branch information
tra committed Aug 9, 2024
1 parent 056308d commit bdf16c9
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 4 deletions.
11 changes: 11 additions & 0 deletions src/game/CAvaraGame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ void CAvaraGame::IAvaraGame(CAvaraApp *theApp) {
statusRequest = kNoVehicleStatus;

nextPingTime = 0;
nextLoadTime = 0;

showNewHUD = gApplication ? gApplication->Get<bool>(kShowNewHUD) : false;
// CalcGameRect();
Expand Down Expand Up @@ -843,6 +844,16 @@ bool CAvaraGame::GameTick() {
nextPingTime = startTime + pingInterval;
}

int randLoadPeriod = Debug::GetValue("rload"); // randomly load a level every `rload` seconds
if (randLoadPeriod > 0) {
if (startTime > nextLoadTime) {
auto p = CPlayerManagerImpl::LocalPlayer();
auto *tui = itsApp->GetTui();
tui->ExecuteMatchingCommand("/rand", p);
nextLoadTime = startTime + 1000*randLoadPeriod;
}
}

// Not playing? Nothing to do!
if (statusRequest != kPlayingStatus)
return false;
Expand Down
1 change: 1 addition & 0 deletions src/game/CAvaraGame.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ class CAvaraGame {

uint32_t nextScheduledFrame;
uint32_t nextPingTime;
uint32_t nextLoadTime;
long lastFrameTime;
Boolean canPreSend;

Expand Down
2 changes: 2 additions & 0 deletions vendor/nanogui/nanogui/screen.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ class NANOGUI_EXPORT Screen : public Widget {
void moveWindowToFront(Window *window);
void drawWidgets();

virtual void removeNotifyParent(const Widget *w) override;

protected:
SDL_Window *mSDLWindow;
uint32_t mWindowID;
Expand Down
13 changes: 11 additions & 2 deletions vendor/nanogui/nanogui/widget.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,16 @@ class NANOGUI_EXPORT Widget : public Object {
/// Set the position relative to the parent widget
void setPosition(const Vector2i &pos) { mPos = pos; }

/// return position of parent or zeros if no parent
Vector2i parentPosition() const {
static Vector2i zeroPos = {};
return mParent ?
(mParent->absolutePosition()) : zeroPos;
}

/// Return the absolute position on screen
Vector2i absolutePosition() const {
return mParent ?
(parent()->absolutePosition() + mPos) : mPos;
return parentPosition() + mPos;
}

/// Return the size of the widget
Expand Down Expand Up @@ -148,6 +154,9 @@ class NANOGUI_EXPORT Widget : public Object {
/// Remove a child widget by value
void removeChild(const Widget *widget);

/// notify other widgets/parents
virtual void removeNotifyParent(const Widget *w);

/// Retrieves the child at the specific position
const Widget* childAt(int index) const { return mChildren[index]; }

Expand Down
15 changes: 13 additions & 2 deletions vendor/nanogui/screen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ bool Screen::cursorPosCallbackEvent(double x, double y) {
}
} else {
ret = mDragWidget->mouseDragEvent(
p - mDragWidget->parent()->absolutePosition(), p - mMousePos,
p - mDragWidget->parentPosition(), p - mMousePos,
mMouseState, mModifiers);
}

Expand Down Expand Up @@ -482,7 +482,7 @@ bool Screen::mouseButtonCallbackEvent(int button, int action, int modifiers) {
if (mDragActive && action == SDL_RELEASED &&
dropWidget != mDragWidget)
mDragWidget->mouseButtonEvent(
mMousePos - mDragWidget->parent()->absolutePosition(), button,
mMousePos - mDragWidget->parentPosition(), button,
false, mModifiers);

if (dropWidget != nullptr && dropWidget->cursor() != mCursor) {
Expand Down Expand Up @@ -694,4 +694,15 @@ bool Screen::handleSDLEvent(SDL_Event &event) {
return false;
}


void Screen::removeNotifyParent(const Widget *w) {
// remove widget from various places before deletion
if (w == mDragWidget) {
mDragWidget = nullptr;
mDragActive = false;
}
mFocusPath.erase(std::remove(mFocusPath.begin(), mFocusPath.end(), w), mFocusPath.end());
Widget::removeNotifyParent(w);
}

NAMESPACE_END(nanogui)
11 changes: 11 additions & 0 deletions vendor/nanogui/widget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,21 @@ void Widget::removeChild(const Widget *widget) {

void Widget::removeChild(int index) {
Widget *widget = mChildren[index];
removeNotifyParent(widget);
mChildren.erase(mChildren.begin() + index);
widget->decRef();
}

// climb the tree and let everyone (Screen) know this object is being removed.
// ideally we would change everything to use shared_ptr<Widget> and maybe some weak_ptr references...
// but, i'm lazy and this code should all get replaced soon (right?)
void Widget::removeNotifyParent(const Widget *w) {
if (mParent) {
mParent->removeNotifyParent(w);
}
return;
}

int Widget::childIndex(Widget *widget) const {
auto it = std::find(mChildren.begin(), mChildren.end(), widget);
if (it == mChildren.end())
Expand Down

0 comments on commit bdf16c9

Please sign in to comment.