Removed duplicate field in CShotgun save data table.

[malortie]

See ValveSoftware/halflife#3206.

[FreeSlave]

Won't it break save files?

[malortie]

It would probably break existing saves, but loading and resaving the game should fix it.

[a1batross]

Better not break save files for no reason.

[a1batross]

It should be compatible with original unmodified Half-Life DLL saves.

[FreeSlave]

I have a proposal.
We create a new branch (called e.g. hl-fixed, not sure about the name, you might have better ideas) to store all fixes that may break Half-Life save-restore or network protocol. There're already some fixes under the macro in this repo (like ones for crowbar and gauss). We enable named fixes in this branch and push any others like the one from this PR. Then we can use this branch to merge mods upon it since we don't strive to maintain save compatibility for mods anyway.

[a1batross]

I've been thinking a lot on this lately. On games that old as Half-Life, some of the bugs are became features. Like it happened with Quake's unintended bunnyhop, Half-Life has func_pushable boosting, wallstrafing and even hated by everyone self-gauss in multiplayer makes sense. Yes, many of these bugs are already fixed in our HLSDK fork and it doesn't make sense to revert them all and try to pursue that original unpatched Half-Life feel.

I see the same problem with keeping compatibility, though it's much easier to decide. If it doesn't load, then it's broken.

In my opinion, such changes can be maintained in same branch and toggled with one macro. For those who want to develop a mod and don't care and those who want their save files back. Like the ReHLDS team does with REHLDS_FIXES macro.

[malortie]

I did several tests by outputting the majority of restored fields to a text file - See the attached files below.
You can use tools such as KDiff3 to help visualize the differences.

reference.txt
test1.txt
test2.txt
test3.txt
test4.txt
test5.txt
test6.txt

reference.txt is the file containing the results after restore with unchanged source code.

Note: The following tests were made using this SDK, which is identical to ValveSoftware/halflife but has been modified to compile under newer versions of VS.

Note: classname string appears to always change, but this is probably expected.

---

Test 1: Remove 2nd field m_flNextReload in CShotgun save data table (same as in PR)

Result:

• All fields correctly restored. ✔️
• All fields offsets remain unchanged.

Test 2: Remove 1st field m_flNextReload in CShotgun save data table

Result:

• All fields correctly restored. ✔️
• All fields offsets remain unchanged.

Test 3: Remove all m_flNextReload fields from CShotgun save data table

Result:

• m_flNextReload does not appear in the list. Therefore, it is not restored, which is expected.
• All other fields are correctly restored. ✔️
• All fields offsets remain unchanged.

Test 4: Declare m_flNextReload member variable after m_iShell

int m_fInReload;
int m_iShell;
float m_flNextReload; // Declared after m_iShell

Result:

• All fields correctly restored. ✔️
• m_flNextReload field offset changed from 192 to 196.
• All other fields offsets remain unchanged.

Test 5: Remove member variable m_flNextReload from CShotgun class

int m_fInReload;
// float m_flNextReload;
int m_iShell;

Result:

• m_flNextReload does not appear in the list, which is expected.
• All fields correctly restored. ✔️
• All field offsets remain unchanged.

Test 6: Swap position of m_flNextPrimaryAttack and m_flTimeWeaponIdle in CBasePlayerWeapon

float	m_flTimeWeaponIdle; // m_flNextPrimaryAttack was originally here.
float	m_flNextSecondaryAttack;
float	m_flNextPrimaryAttack; // m_flTimeWeaponIdle was originally here.

Result:

• All fields correctly restored. ✔️
• m_flTimeWeaponIdle field offset changed from 148 to 140.
• m_flNextPrimaryAttack field offset changed from 140 to 148.
• All other fields offsets remain unchanged.

---

In this case all fields are correctly restored, even after removing variables and changing offset.

Note: these results are only for CShotgun. I have not tested global fields or with other entities, so I am not sure if there could be situations where breaking changes would occur.

[malortie]

Below are additional details on Save/Restore analysis.

In the CRestore::ReadField, it compares the field name defined in the DLL save data table against the one from the saved file (without case sensitivity).

https://github.com/FWGS/hlsdk-xash3d/blob/215ac04f7094880c04d5eb40c9c63dd8853b6ed8/dlls/util.cpp#L2160

then sets the following pointers to output and input:

https://github.com/FWGS/hlsdk-xash3d/blob/215ac04f7094880c04d5eb40c9c63dd8853b6ed8/dlls/util.cpp#L2166-L2167

pBaseData points to the DLL entity instance address (this in this case).
pTest->fieldOffset is the offset of the DLL member variable address, relative to pBaseData. This is determined at compile time using offsetof.

https://github.com/FWGS/hlsdk-xash3d/blob/215ac04f7094880c04d5eb40c9c63dd8853b6ed8/engine/eiface.h#L362-L363

Since it looks for the field name, this allows the code to correctly apply the input value relative to pBaseData.

So far of my understanding, the following changes would break compatibility with older saves:

• Renaming a registered DLL variable
• Changing the type of a registered DLL variable to a new type different in size e.g. float -> Vector
• Changing the field type in the save data table to a new type different in size e.g. FIELD_FLOAT -> FIELD_VECTOR

[FreeSlave]

Nice research. To my understanding even the resaved save (with the proposed change) should stay compatible with original, so it's compatible on both ends.