Skip to content

Conversation

@Goober5000
Copy link
Contributor

Several related fixes:

  1. Copy the actual ship name, not the display name, to killer_parent_name for two reasons:
    a. Since killer_parent_name is used to look up the ship, it must be the actual ship name, not the display name.
    b. Copying the display name can cause a buffer overflow because display names can be longer than 31 characters.
  2. The display name functionality is moved to the more appropriate location in player_generate_death_message().
  3. Remove player_get_killer_weapon_name() which only served as a proxy check for whether the weapon index was used or not.
  4. Use ship_registry_get() rather than ship_name_lookup().
  5. Add comments to clarify the use of killer_parent_name.

Fixes a bug reported in Discord.

Several related fixes:
1. Copy the actual ship name, not the display name, to `killer_parent_name` for two reasons:
  1a. Since `killer_parent_name` is used to look up the ship, it must be the actual ship name, not the display name.
  1b. Copying the display name can cause a buffer overflow because display names can be longer than 31 characters.
2. The display name functionality is moved to the more appropriate location in `player_generate_death_message()`.
3. Remove `player_get_killer_weapon_name()` which only served as a proxy check for whether the weapon index was used or not.
4. Use `ship_registry_get()` rather than `ship_name_lookup()`.
5. Add comments to clarify the use of `killer_parent_name`.
@Goober5000 Goober5000 added the fix A fix for bugs, not-a-bugs, and/or regressions. label Jul 4, 2023
@Goober5000 Goober5000 added this to the Release 23.2 milestone Jul 4, 2023
Copy link
Member

@wookieejedi wookieejedi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Goober5000 Goober5000 merged commit 859534a into scp-fs2open:master Jul 7, 2023
@Goober5000 Goober5000 deleted the ship_killer_name_fix branch July 7, 2023 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A fix for bugs, not-a-bugs, and/or regressions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants