Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed the problem with escorts (I think). #882

Merged
merged 7 commits into from
Jan 21, 2020
Merged

Conversation

onpon4
Copy link
Contributor

@onpon4 onpon4 commented Jan 20, 2020

Not tested thoroughly yet, but I think this should fix the problem of
escorts causing segfaults when their origin ship dies. A cursory test
shows that this seems to work.

This also causes pilot.setLeader to remove "deployed" status from the
origin ship.

Not tested thoroughly yet, but I think this should fix the problem of
escorts causing segfaults when their origin ship dies. A cursory test
shows that this seems to work.

This also causes pilot.setLeader to remove "deployed" status from the
origin ship.
src/pilot.c Outdated Show resolved Hide resolved
@ids1024
Copy link
Member

ids1024 commented Jan 20, 2020

Calling assert broke the Travis build. If you want to do that, include assert.h.

src/player.h Outdated Show resolved Hide resolved
@onpon4
Copy link
Contributor Author

onpon4 commented Jan 20, 2020

I've added an include for assert.h and changed player_dt_default to a function.

I'm not sure about switching up the way dockpilot and dockslot are stored, it's not going to improve efficiency any (and in fact can make it worse because it adds more looping). That it involves passing pointers around is true enough but failure to clean up the other values properly can cause bugs too (just bugs that would be less obvious like choosing the wrong ship as a dockpilot and then trying to dock on a slot that it doesn't belong in).

@onpon4
Copy link
Contributor Author

onpon4 commented Jan 21, 2020

Now, if every single ship that could possibly have the destroyed pilot as a dockslot will always be in the escort list, that can be used to reduce looping a bit. That looks rather flimsy to me, though, and I don't think that's really worth it when the pilots have to be looped through all the time anyway.

@onpon4
Copy link
Contributor Author

onpon4 commented Jan 21, 2020

There we go, switched it up to use ints instead of pointers.

@ids1024
Copy link
Member

ids1024 commented Jan 21, 2020

Do you see my point about just removing the loop from pilot_destroy, or am I missing something? All that code does is setting dockpilot and dockslot to 0 and -1. Without that loop, those variables will be set the same way the next time pilot_getDockSlot() is called.

Otherwise, the changes here look good. 👍

@onpon4
Copy link
Contributor Author

onpon4 commented Jan 21, 2020

Ahhhhhhh, I see what you're saying. That does make sense come to think of it. Lemme do that...

@onpon4
Copy link
Contributor Author

onpon4 commented Jan 21, 2020

There we go, I've pushed that through. Seems to work.

@ids1024 ids1024 merged commit 85a373f into naev:master Jan 21, 2020
@onpon4 onpon4 deleted the bugfix2 branch January 21, 2020 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants