Skip to content
This repository has been archived by the owner on Apr 29, 2024. It is now read-only.

fix crash on Ship_HullDamage call (event_name "ShpHullDamage") #369

Merged
merged 1 commit into from May 19, 2022

Conversation

q4a
Copy link
Contributor

@q4a q4a commented May 19, 2022

How to reproduce crash:
Load attached save and fight with enemy in sea.
Open_sea-SAVE.zip
Crash sometimes happens when cannonballs hit your ship.

How it looks like in debugger:
unknown

Copy link
Member

@mitrokosta mitrokosta left a comment

Choose a reason for hiding this comment

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

This bug seems to have been around for a while, but only Hammie's changes made it apparent.

@mitrokosta mitrokosta merged commit 88cc21c into storm-devs:develop May 19, 2022
@espkk
Copy link
Member

espkk commented May 19, 2022

TEHO that I have in steam uses that additional 'a' parameter 🤔

Is this a matter of bad versioning? @mitrokosta

@q4a q4a deleted the fix-hull-dmg branch May 20, 2022 00:59
@mitrokosta
Copy link
Member

mitrokosta commented May 20, 2022

TEHO that I have in steam uses that additional 'a' parameter 🤔

Is this a matter of bad versioning? @mitrokosta

No, it's not. The only usage is aref aCharacter = GetEventData(); // владелец ядра (от кого прилетело)
But then this aCharacter is never used. q4a updated the scripts in sd-teho-public, removing this usage.
Btw with Hammie's changes this event would crash the game anyway because of parameter mismatch. So nothing really changes if you use newer versions of the engine.

@espkk
Copy link
Member

espkk commented May 20, 2022

TEHO that I have in steam uses that additional 'a' parameter 🤔
Is this a matter of bad versioning? @mitrokosta

No, it's not. The only usage is aref aCharacter = GetEventData(); // владелец ядра (от кого прилетело) But then this aCharacter is never used. q4a updated the scripts in sd-teho-public, removing this usage. Btw with Hammie's changes this event would crash the game anyway because of parameter mismatch. So nothing really changes if you use newer versions of the engine.

I can see those

	int iHullNum = GetEventData(); // номер ноды в которую прилетело
	float x = GetEventData();
	float y = GetEventData();
	float z = GetEventData();
	float fDamage = GetEventData();
	aref rCharacter = GetEventData(); // в кого прилетело
	aref aCharacter = GetEventData(); // владелец ядра (от кого прилетело)
	string HullName = GetEventData(); // имя ноды в которую прилетело

both rCharacter and aCharacter are aref. iBallOwner (int) is obviously invalid here, though reading both attributes from stack would cause the same kind of crash

@mitrokosta
Copy link
Member

mitrokosta commented May 20, 2022

TEHO that I have in steam uses that additional 'a' parameter 🤔
Is this a matter of bad versioning? @mitrokosta

No, it's not. The only usage is aref aCharacter = GetEventData(); // владелец ядра (от кого прилетело) But then this aCharacter is never used. q4a updated the scripts in sd-teho-public, removing this usage. Btw with Hammie's changes this event would crash the game anyway because of parameter mismatch. So nothing really changes if you use newer versions of the engine.

I can see those

	int iHullNum = GetEventData(); // номер ноды в которую прилетело
	float x = GetEventData();
	float y = GetEventData();
	float z = GetEventData();
	float fDamage = GetEventData();
	aref rCharacter = GetEventData(); // в кого прилетело
	aref aCharacter = GetEventData(); // владелец ядра (от кого прилетело)
	string HullName = GetEventData(); // имя ноды в которую прилетело

both rCharacter and aCharacter are aref. iBallOwner (int) is obviously invalid here, though reading both attributes from stack would cause the same kind of crash

This is why we removed aref aCharacter = GetEventData(); // владелец ядра (от кого прилетело) line

@espkk
Copy link
Member

espkk commented May 20, 2022

I see, that's exactly my question

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants