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

LSL3DEMO: Selectors are all zero #6

Closed
EricOakford opened this issue Jul 18, 2020 · 9 comments
Closed

LSL3DEMO: Selectors are all zero #6

EricOakford opened this issue Jul 18, 2020 · 9 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@EricOakford
Copy link

The LSL3 demo is missing its selector table. No worries, I can grab the one from the full game.

However, there's a more serious issue here - when deompiling the game scripts (system scripts are unaffected), the decompiler gives every selector in code as zero! Thus, they're all called "species". This doesn't occur when looking at the disassembled scripts in SCI Viewer.

@Kawa-oneechan Kawa-oneechan added the bug Something isn't working label Jul 18, 2020
@Kawa-oneechan Kawa-oneechan self-assigned this Jul 18, 2020
@Kawa-oneechan
Copy link
Owner

The example I've been working with is (super init:) in room 200.

35 57            ldi 57
36               push 
39 00            pushi 0 // species
57 36 04         super Rm 4 

In DecompilerNew.cpp line 1507, it's supposed to return that 57, but it gives a 00 instead. In the real LSL3, I get 57 from this matching code:

39 57            pushi 57 // init
76               push0 
57 36 04         super Rm 4 

Notice how Companion's disassembler put a comment on pushi 0 in the demo, but on pushi 57 in the real game and not push0?

Clearly, the decompiler seems to prefer pushi for sends, but that's as far as I could get.

@Kawa-oneechan Kawa-oneechan added the help wanted Extra attention is needed label Jul 18, 2020
@Kawa-oneechan
Copy link
Owner

If I was bored enough, I'd go through what few non-system scripts there are and manually fix up the selectors myself.

@Kawa-oneechan
Copy link
Owner

I fixed it! The whole thought process is on my twitter but here's the beef: https://twitter.com/NoxicoDev/status/1284492738831044613

And here's a direct link to today's build, with this fix included: http://helmet.kafuka.org/sci/SCICompanion.exe

@EricOakford
Copy link
Author

As it turns out, the fix wasn't perfect. While many selectors are now correctly identified, there are still many more identified as species.

@Kawa-oneechan
Copy link
Owner

Well shit. Gonna look for "species" then, see what else is up.

@Kawa-oneechan
Copy link
Owner

Found it! pNodePrevious was set, but did not point to the LDI opcode I expected. So I changed the logic around a bit and now the only instances of species in my decompilation are in Obj::isKindOf and isMemberOf. As it should be.

Direct download in the same place as before.

@Kawa-oneechan Kawa-oneechan reopened this Jul 31, 2020
@Kawa-oneechan
Copy link
Owner

@EricOakford, does it work?

@Kawa-oneechan
Copy link
Owner

I'm just going to assume it works then.

@sluicebox
Copy link

I know this is a very old thread, but I want to share that this is a fun bug with decompiler implications.

What makes lsl3-demo different? It was compiled without optimizations. Sierra passed the -z option to SC.EXE. That revealed that this decompiler was hard-coded to expect optimized instructions in precise locations. Yikes!

Optimizations make decompiling SCI hard, so the last thing I'd expect is for a decompiler to choke on unoptimized instructions — those are supposed to be the easy ones!

I am selfishly delighted because Phil's articles convinced me that "reverse instruction consumption" (my term) was too complicated to fit in my head, so I didn't do it in my decompiler. I also suspected (hoped!) that it was unnecessary complexity, and not suited to handling SC.EXE's optimizations and edge cases. And now it turns out that it didn't handle the unoptimized cases!

I don't really know how this decompiler works, so maybe reverse instruction consumption is a few shallow bugs away from working flawlessly, but I suspect it is the cause of most silent inaccuracies and difficult to fix. Compiler optimizations and bugs create messy situations.

Alternatively: my decompiler consumes instructions and control flow nodes from start to end, and maintains a small SCI VM-like state for expressions. It's easier to reason about, avoids a dependency tree data model, and naturally handles whatever pushes and loads the compiler spits out; optimized or not.

In other words, I skipped all this =)

Pretty neat that two such different approaches can get good results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants