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

Improve Pbg3Archive accuracy #321

Merged
merged 5 commits into from
Nov 15, 2024
Merged

Conversation

KSSBrawl
Copy link
Contributor

@KSSBrawl KSSBrawl commented Nov 14, 2024

Pbg3Archive::Load 88% -> 98%
Pbg3Archive::FindEntry 82% -> 100%
Pbg3Archive::GetEntrySize 84% -> 100%

There is currently an issue with the codegen for Pbg3Parser's scalar deleting destructor that is preventing Pbg3Archive::Load and Pbg3Archive::ParseHeader from matching.

@KSSBrawl KSSBrawl changed the title Improve Pgb3Archive accuracy Improve Pbg3Archive accuracy Nov 14, 2024
{
return -1;
}
// Why was this here? It's certainly not present in the assembly.
Copy link
Member

Choose a reason for hiding this comment

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

The PBG3 code wasn't a very faithful reimpl - I was more concerned in having something that worked than something that matched, so I could move on to the actual game logic faster.

So in that context, this might have been put here as a workaround or something while working on it. Anyways, you can delete these lines - they're indeed not from the original ^^.

Copy link
Member

Choose a reason for hiding this comment

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

(I might also have used various open source PBG3 impls as references, like thtk, which may be where this came from? Not sure)

@roblabla
Copy link
Member

Very nice work!

@roblabla roblabla merged commit 8de1323 into happyhavoc:master Nov 15, 2024
6 checks 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.

2 participants