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

Move all usage of dword_587000_155144 #713

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

blmarket
Copy link
Contributor

@blmarket blmarket commented Aug 1, 2024

Hi, can you take a quick look and see it makes sense? Would like to check before working on adding unit test & etc.

Context

Working on audio effects, trying to make small incremental changes.

dword_587000_155144 is actually a pointer to a struct defined globally, storing 2 linked lists and a counter variable.

Next steps

There will be more changes defining Struct88 and Struct264 which are actually 88 and 264 byte sized struct dynamically allocated.

Required sign-off

  • I confirm that my PR does not contain any commercial or protected assets and/or source code.
  • I agree in advance that my codes will be licensed automatically under the Apache License or similar BSD/MIT-like
    open source licenses in case if OpenNox Project will adopt such a non-GPL license in the future.

func sub_4875D0(a1 **Struct264) *Struct264 {
if *a1 != nil {
// FIXME: should be no cast
*a1 = (*Struct264)(unsafe.Pointer((*a1).getList().Next()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks we need a better type definition for list items...

@blmarket
Copy link
Contributor Author

blmarket commented Aug 1, 2024

Few items to discuss:

  1. I put this one in opennox package instead of subpackage of legacy due to list implementation, but that decision made the code not to be isolated. Can we extract list definition (listHead, listItem etc.) to a separate package which allows this PR code to be in another package?
  2. Also it seems we have problem supporting list items in pure safe Go. Nox's list implementation looks like: there is a head which has only 3 pointers, and there are actual struct which begins with 3 pointers, but rest being used to store more information. This makes typing complicated - pointer can points to either head or full item, and can't avoid using unsafe to deal with them.

Current Go code is not equivalent with corresponding C code:
https://github.com/noxworld-dev/opennox/blob/dev/src/legacy/GAME1_1.c#L5178

Logical explanation:

Only the listHead is guaranteed to have field_2 set to itself, while
list members does not have such guarantee. So the best way to check
the listItem is to check the current pointer is same with field_2
(or head)
@dennwc
Copy link
Collaborator

dennwc commented Aug 11, 2024

Hey @blmarket ! I'll check it out soon, was on vacation.

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