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

Shadow of the Erdtree Compatibility #931

Merged
merged 6 commits into from
Jul 19, 2024

Conversation

vawser
Copy link
Contributor

@vawser vawser commented Jun 30, 2024

  • Added new Paramdex stuff for ER
  • Added new Paramdex stuff for DS3
  • Added new Paramdex stuff for AC6
  • Added MSB changes for ER
  • Added MSB changes for DS3
  • Added MSB changes of AC6
  • Added "Param Validation", "Paramdef Validation" and "Map Validation" tests for checking the param padding, paramdef accuracy and MSB accuracy.
  • Fixed bug with eventParam save in ER where it would fail due to the condition checking for sysParam != null instead of eventParam

vawser added 2 commits June 30, 2024 04:07
- Added new Paramdex stuff for ER
- Added new Paramdex stuff for DS3
- Added new Paramdex stuff for AC6
- Added MSB changes for ER
- Added MSB changes for DS3
- Added MSB changes of AC6
- Added "Param Validation", "Paramdef Validation" and "Map Validation" tests for checking the param padding, paramdef accuracy and MSB accuracy.
@@ -172,6 +180,9 @@ internal override Event ReadEntry(BinaryReaderEx br)
case EventType.RetryPoint:
return RetryPoints.EchoAdd(new Event.RetryPoint(br));

case EventType.AreaTeam:
return AreaTeams.EchoAdd(new Event.AreaTeam(br));

Copy link
Collaborator

Choose a reason for hiding this comment

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

Happy to see updates here. Not sure if this what tk was bemoaning but it'll help.

@@ -782,17 +784,17 @@ public class UnkStruct1
/// <summary>
/// Unknown.
/// </summary>
public uint[] DisplayGroups { get; private set; }
public uint[] DisplayGroups { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

with all the choices to private indexes for sanity, why are we making these setters public?
being an array, it's already an immutable reference to a correctly-size mutable array? maybe that's not how c# handles it idk

Copy link
Contributor

@ividyon ividyon Jul 9, 2024

Choose a reason for hiding this comment

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

Main use case is XML serialization as it would fail on this
If this is handled by some kind of setter method, it should be overriding this default setter, and the propertyshould be backed by a private field


/// <summary>
/// The map to load when on this collision.
/// </summary>
public byte[] MapID { get; private set; }
public byte[] MapID { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

again privacy thing.
I'm guessing this is a convenience for setting to a mapid.
Perhaps we can write in the length-check in the setter method?

Copy link
Contributor

Choose a reason for hiding this comment

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

Main use case is XML serialization as it would fail on this
If this is handled by some kind of setter method, it should be overriding this default setter, and the propertyshould be backed by a private field

/// Unknown.
/// </summary>
[IgnoreProperty]
public short UnkT00 { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

ideally this is placed at the top of the class, in regular field order

@@ -15,6 +15,7 @@
<PackageReference Include="Google.Protobuf" Version="3.25.2" />
<PackageReference Include="Grpc.AspNetCore" Version="2.60.0" />
<PackageReference Include="Grpc.Net.Client" Version="2.60.0" />
<PackageReference Include="ZstdNet" Version="1.4.5" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

soapstone is requiring it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the new compression for regulation.bin

bw.WriteInt32(8);
*/
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still hopeful we'll figure this out, though it seems more and more likely we'll need to inject some hardcore ghidra user like chain with a few shots of caffeine

@@ -45,7 +45,8 @@
<Field Def="f32 emissiveValueEnd"></Field>
<Field Def="f32 emissiveTime"></Field>
<Field Def="u8 bIntpEnable"></Field>
<Field Def="dummy8 pad_01[3]"></Field>
<Field Def="u8 unknown_0x59"></Field>
<Field Def="dummy8 pad_01[2]"></Field>
Copy link
Collaborator

Choose a reason for hiding this comment

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

one of these days we'll do due diligence and check it's a byte and not a short the first time. But I'm happy with this for now.

@@ -27,6 +27,7 @@
<Field Def="s32 priority = 3" />
<Field Def="f32 execInvalidTime" />
<Field Def="u8 execButtonCircle = 1" />
<Field Def="dummy8 pad4[3]" />
<Field Def="u8 unknown_0x45" />
<Field Def="dummy8 pad4[2]" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good finds here in ds3, gracias

@@ -1397,7 +1397,12 @@
</Field>
<Field Def="s32 finalDamageRateId">
</Field>
<Field Def="dummy8 pad7[12]">
</Field>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This dummy I'm fairly sure came after finaldamagerate, so these could just be renamings.
Also it may be worth checking if finaldamagerate was this as well, since it showed up in like 1.06 or 1.07 and wasn't in original paramdefs

<Unicode>True</Unicode>
<FormatVersion>203</FormatVersion>
<Fields>
<Field Def="s32 Unknown0" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

prefer lowercase unknown, and byte offset than number

@@ -6,15 +6,15 @@
<Unicode>True</Unicode>
<FormatVersion>203</FormatVersion>
<Fields>
<Field Def="u8 enableRaytraceAO"></Field>
<Field Def="u8 enableRaytraceShadows"></Field>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this was confirmed as community gaslighting wasn't it?

<Unicode>True</Unicode>
<FormatVersion>203</FormatVersion>
<Fields>
<Field Def="s32 Unknown0" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

lowercase+offset, many such comments apply in places other than where they first appear

@@ -264,9 +264,14 @@
<Maximum>1</Maximum>
<SortID>2100</SortID>
</Field>
<Field Def="dummy8 pad1:7">

<Field Def="dummy8 pad1_old:7" RemovedVersion="11210015" />
Copy link
Collaborator

@Philiquaz Philiquaz Jun 30, 2024

Choose a reason for hiding this comment

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

Is there particular reason to rename old field? Curious, since it's also a relic of original paramdef. Some util I'm guessing is expecting distinct names across versions?
My preference is for pad_new, though I don't think this change is a stopper lol

<Field Def="u8 swordArtsTypeNew" FirstVersion="10701000" RemovedVersion="11210015" />
<Field Def="dummy8 pad[1]" RemovedVersion="11210015" />

<Field Def="u16 swordArtsTypeDlc" FirstVersion="11210015" />
Copy link
Collaborator

@Philiquaz Philiquaz Jun 30, 2024

Choose a reason for hiding this comment

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

Nord confirmed that swordArtsTypeNew should've been u16 since 10701000
I fucked that one up

<pad AltName=""/>

<swordArtsTypeNew AltName="Sword Arts Type" Wiki="Offset for TAE animation. 0 is a600. To pass to the behavior script to determine which swashbuckler" />
<swordArtsTypeDlc AltName="Sword Arts Type" Wiki="Offset for TAE animation. 0 is a600. To pass to the behavior script to determine which swashbuckler" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

check def comment

100116315 Blade of Calling
100116316 Blade of Calling
100116317 Blade of Calling
100101100 Fire Knight's Shortsword / Blade Knife -
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm assuming these dashes are placeholder for labelling individual moves?

@@ -179,7 +179,6 @@
21910000 Radagon - Crucified (Unscaled)
22000000 Elden Beast
22000078 Elden Beast
22000178 Elden Beast
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing removals in this file are because they don't appear, than that they don't match chr-id conventions?

@@ -3536,7 +3536,6 @@
30917003 Cough up your coin, all of it!
30917100 Hm? Wait, don't tell me, is that you?
30917101 Oh, cripes, please, wait!
30917102 I surrender! I surrender, I swear!
Copy link
Collaborator

Choose a reason for hiding this comment

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

patches my beloved...

param WepAbsorpPosParam: modified && !added: reserve: = vanillafield reserve
11210015L
1.12.1 - (SwordArtsParam) Set swordArtsTypeDlc to Vanilla v1.12.1 values
Copy link
Collaborator

Choose a reason for hiding this comment

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

see def comment

11210015L
1.12.1 - (WorldMapPointParam) Set unknown_0xb7 to Vanilla v1.12.1 values
param WorldMapPointParam: modified && !added: unknown_0xb7: = vanillafield unknown_0xb7
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can coalesce as unknown_0xb[789] if we use = vanilla; (I think previous commands may have been written before that shortcut)

11210015L
1.12.1 - (WepAbsorpPosParam) Set unknown_0x54 to Vanilla v1.12.1 values
param WepAbsorpPosParam: modified && !added: unknown_0x54: = vanillafield unknown_0x54
Copy link
Collaborator

Choose a reason for hiding this comment

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

unknown_0x5[567]

11210015L
1.12.1 - (SpEffectVfxParam) Set unknown_0x96 to Vanilla v1.12.1 values
param SpEffectVfxParam: modified && !added: unknown_0x96: = vanillafield unknown_0x96
Copy link
Collaborator

Choose a reason for hiding this comment

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

unknown_0x9[6789a]

11210015L
1.12.1 - (SpEffectParam) Set unknown_0x352_6 to Vanilla v1.12.1 values
param SpEffectParam: modified && !added: unknown_0x352_6: = vanillafield unknown_0x352_6
Copy link
Collaborator

Choose a reason for hiding this comment

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

unknown_0x35(2_[67]|3_[012345])

param ToughnessParam: modified && !added: unk2: = vanillafield unk2
Copy link
Collaborator

Choose a reason for hiding this comment

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

param SwordArtsParam: modified: swordArtsTypeNew: = field swordArtsType will stop functioning if the name is changed again (should re-check if other unks were identified too)

11210015L
1.12.1 - (AssetGeometryParam) Set Reserve_0 to Vanilla v1.12.1 values
param AssetGeometryParam: modified && !added: Reserve_0: = vanillafield Reserve_0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to see we don't have a heap of fields being actively moved.
Are there any additions where we should modify modified && added to = 1 for example, like damagerate fields that are otherwise going to be initialised as 0?
I don't see any new refs like we had for finaldamagerate, but same story.

vawser and others added 4 commits June 30, 2024 13:42
- Changed DecompressDCPZSTD function name to DecompressDCXZSTD
- Re-privatized some MSBE.Part properties
- Added in proper paramdef names for the new 1.12.1 params
- Added in some ER Names that were missed
- Restored some ER Names that were accidently removed
- Actually included the restored names
- Updated Paramdex to latest changes.
@Philiquaz Philiquaz merged commit 97bc264 into soulsmods:master Jul 19, 2024
1 check passed
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.

3 participants