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

Decompile: prototype | video | vol-h | air-h | nav-enemy-h | rigid-body-h #575

Merged
merged 15 commits into from
Jun 13, 2021

Conversation

xTVaser
Copy link
Member

@xTVaser xTVaser commented Jun 10, 2021

The last 3 header files have issues compiling, but it decompiles fine. They are just mostly simple type-defs so this seems strange

Copy link
Collaborator

@water111 water111 left a comment

Choose a reason for hiding this comment

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

I think those three files compile fine, but aren't decompiled as part of the offline test because we're using the ENGINE.CGO file, and those files are only found in GAME.CGO. If you change "CGO/ENGINE.CGO" to "CGO/GAME.CGO" inside of offline_test_main.cpp, does it work?

; :flag-assert #x900000020
; )
(deftype air-box (structure)
((vecs vector 2 :offset-assert 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is probably :inline.

(set! (-> *video-parms* set-video-mode) #t)
(set-hud-aspect-ratio (get-aspect-ratio) arg0)
(when *progress-process*
(let ((s4-0 (method-of-object (-> *progress-process* 0) dummy-23)))
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 that dummy-23 takes an additional argument here, based on how this came out.

It might be that dummy-23 doesn't use this argument, but I think adding it would make this code come out nicer.

)
(set-hud-aspect-ratio arg0 (the-as video-parms (get-video-mode)))
(when *progress-process*
(let ((s4-1 (method-of-object (-> *progress-process* 0) dummy-23)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, I think this will look nicer if dummy-23 took another argument


;; definition for function set-video-mode
;; INFO: Return type mismatch int vs none.
(defun set-video-mode ((arg0 video-parms))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think arg0 here is likely a symbol here and not video-parms. It's compared to 'ntsc and 'pal. Both the compiler/decompiler do very little type checking for comparing things for equality, but it doesn't make sense to compare a structure to a symbol.

(-> *font-default-matrix* vector 0 x)
(-> *video-parms* relative-x-scale)
)
(set-hud-aspect-ratio arg0 (the-as video-parms (get-video-mode)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

The set-hud-aspect-ratio function probably takes a symbol as its second argument, not a video-parms.

(s4-1 (method-of-object s5-1 TODO-RENAME-23))
)
(get-video-mode)
(s4-1 s5-1 arg0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, this still doesn't look quite right. I'd expect this to look something like:

(TODO-RENAME-23 (-> *progress-process* 0) s5-1 arg0 (get-video-mode))

I think this function takes an additional argument.

This is a case where it might help to look at the assembly for the method call. The GOAL compiler generally evaluates expressions left-to-right:

First, it gets the object that it will call the method on: (-> *progress-process*)

    lw v1, *progress-process*(s7)
    lwu s5, 0(v1)

Then, it gets the method from the method table:

    lwu v1, -4(s5)            ;; (-> s5 type)
    lwu s4, 108(v1)           ;; (-> v1 method-table 23)

The first two arguments are just variables and don't require any code to evaluate. So this is code for the third argument: (get-video-mode)

    lw t9, get-video-mode(s7) ;; (set! t9 get-video-mode)[39]
    jalr ra, t9               ;; (call!)[40]
    sll v0, ra, 0

Next, it moves the result into the argument register:

    or a2, v0, r0             ;; (set! a2 v0)[41]

And the final step is to insert a bunch of moves that move existing variables into the right registers (the compiler may omit these if the variable happens to be in the right register already):

    or t9, s4, r0             ;; (set! t9 s4)[42]
    or a0, s5, r0             ;; (set! a0 s5)[43]
    or a1, gp, r0             ;; (set! a1 gp)[44]

and then call the method

    jalr ra, t9               ;; (call!)[45]
    sll v0, ra, 0

@coveralls
Copy link

coveralls commented Jun 12, 2021

Pull Request Test Coverage Report for Build 931876792

  • 1 of 2 (50.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 69.561%

Changes Missing Coverage Covered Lines Changed/Added Lines %
test/offline/offline_test_main.cpp 0 1 0.0%
Totals Coverage Status
Change from base Build 931572654: 0.0%
Covered Lines: 36420
Relevant Lines: 52357

💛 - Coveralls

@water111 water111 merged commit 506b5d8 into open-goal:master Jun 13, 2021
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