lamadaemon / phigros-save-manager

A Typescript library for parse and modify the save file of game Phigros
14 stars 0 forks source link

Severe bugs #4

Closed WarfarinBloodanger closed 4 months ago

WarfarinBloodanger commented 4 months ago

Hi there... Yet I don't have my own PC to make pull requests so I can just issue it here....

In your project I found some severe bugs that mess everything up, and I tried to fix them on my own.

1. Malformed Binary Data?

In the structure LevelRecord there is a field called unknown value. In binary data, it takes a byte in every pieces of records. 图片

Actually it matters. It represents the size of that piece of record. After reading all the necessary fields (e.g. score, accuracy), you are supposed to directly move the cursor to the next piece, skipping rest of bytes (I don't know why there they are , but they do exist. Maybe they represents the performance of Legacy chart or something).

But no skips are seen in readFromDefinition. Everything will be messed up since the program doesn't move the cursor to where it should be. To handle it, I made an if branch in the code that acts as what I mentioned above. The modified code looks like this: 图片

Similarly, writeFromDefiniton should take this byte into consideration, too: 图片

By making it a fixed length with zero-padding, everything can be formed well. However I am not sure that whether discarding those rest-bytes is okay or not, for they may represent meaningful data or something, so before they are checked no final resolution should be made in my point of view.

2. Incorrect Results after Reading?

I found that performances of different levels in one song all come to the same... See my readFromDefinition. You should not directly assign definition.base to the local variable obj... Instead you can use {...definition.base} to make clones, or they all refers to one particular object... That's why everything became same...

3. Details That Cause Errors?

I am not meaning to criticize anyone. I sincerely appreciate the repo owner who made save-managements available for us. I just want to suggest that code be tested before publishing them as a public repo.

Pardon my poor English expressions.

lamadaemon commented 4 months ago

Thank you so much for your detailed report! A bug fix will be delivered by Saturday! Since I am lacking of testing data so I had only tested the features with my own save file.

lamadaemon commented 4 months ago

They should be fixed in 0b3d61f. However, as following what you said, I will test around with the new code before I publish the next version to NPM registry.

lamadaemon commented 4 months ago

Since 1.0.8 was published to NPM, I'll close this issue. If the bug wasn't properly fixed, please reopen.