Skip to content

fix: prevent segfault when template does not exist#2113

Merged
daid merged 1 commit intodaid:masterfrom
Piglit:fix-template
May 25, 2024
Merged

fix: prevent segfault when template does not exist#2113
daid merged 1 commit intodaid:masterfrom
Piglit:fix-template

Conversation

@Piglit
Copy link
Copy Markdown
Contributor

@Piglit Piglit commented May 23, 2024

setTemplate now produces an error, if the chosen template does not exist. But the game no longer crashes with a segfault.

setTemplate now produces an error, if the chosen template does not
exist. But the game no longer crashes with a segfault.
@daid
Copy link
Copy Markdown
Owner

daid commented May 24, 2024

I'm quite sure this moves segfaults to other spots, there are quite some places in the code that assume that the template property is set.

@Piglit
Copy link
Copy Markdown
Contributor Author

Piglit commented May 24, 2024

Actually it produces quite nice errors and defined behaviour, since objects without templates get deleted in ShipTemplateBasedObject::update:

ERROR: Failed to find ship template: AtlantisXX
ERROR: Failed to find ship template: Adder MK5XX
ERROR: Failed to find ship template: 
ERROR: ShipTemplateBasedObject with ID 241 lacked a template, so it was destroyed.
ERROR: Failed to find ship template: 
ERROR: ShipTemplateBasedObject with ID 246 lacked a template, so it was destroyed.

@daid daid merged commit 427ac0a into daid:master May 25, 2024
@daid
Copy link
Copy Markdown
Owner

daid commented May 25, 2024

In that case, it's mostly fine. There is still a chance that it runs into a crash because other code doing something with the object before the update runs, but those are then extreme edge cases

Tsht pushed a commit to Tsht/EmptyEpsilon that referenced this pull request Oct 12, 2024
setTemplate now produces an error, if the chosen template does not
exist. But the game no longer crashes with a segfault.

Co-authored-by: Pithlit <piglit@mail.de>
@Piglit Piglit deleted the fix-template branch July 14, 2025 11:47
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