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

fix(ItemDefinition): fix read/write #57

Closed
wants to merge 2 commits into from
Closed

fix(ItemDefinition): fix read/write #57

wants to merge 2 commits into from

Conversation

kurtekat
Copy link

@kurtekat kurtekat commented Feb 6, 2024

when using episode 6 format, ReqVg values are being assigned to Unknown, which doesn't seem right. ItemCreate.ini variants are the best example.

@matigramirez
Copy link
Owner

matigramirez commented Feb 6, 2024

I just checked with a my own EP6 Item.SData and the ReqVg field is being read properly:

shStudio:
image

Parsec exported json:
image

Item.SData file I used:
Item.zip

Can you provide an Item.SData file where the field isn't read properly?

@kurtekat
Copy link
Author

kurtekat commented Feb 6, 2024

Can you provide an Item.SData file where the field isn't read properly?

absolutely.

Item.zip

Capture\

Capture2

this caught my attention when i was checking crafting hammers for someone that said the rate increase wasn't showing in the client. the items are 102074 and 102075. everything looked fine on my end.

i attached a debugger to my client and watched it copy the value, so i didn't understand what was happening. he was using the same exe, but not the same data.

i see you don't have the hammers in your data, but you do have teleport scrolls. when i use 101102-101109 with your data, the teleport location names do not appear, but they do appear with mine. i'm sure you know the variant is the gatekeeper id, which explains the difference.

@matigramirez
Copy link
Owner

I remember having trouble with these fields in particular because I couldn't get it done properly for some time which could have been the result of using the wrong test subject.

Back then I did some reverse engineering on the ps182 game client and ReqLuc's type was int32 instead of int16. If what you're saying is true, then the Unknown (int16) field is simply the 2 most significant bytes of ReqLuc, which just so happen to be always null.

I need to do some further investigation. For the time being I won't merge nor close this PR.

@matigramirez
Copy link
Owner

@kurtekat if you can please check if #58 fixes your issue

@kurtekat
Copy link
Author

kurtekat commented Feb 7, 2024

@matigramirez it looks good to me. thanks. it solved my issue. the output file is identical to the original.

@kurtekat
Copy link
Author

kurtekat commented Feb 7, 2024

i forgot to mention the edit on line 302. when i was comparing ep5 data and wrote the SData from json, i noticed the output bytes were different than the original. i figured you meant to write Range as a byte for ep5 or less, since that's how it's read.

@matigramirez
Copy link
Owner

You're right, that should have been caught by a unit test... time to review that too.

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