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

Some fixes and features to make it easier to access data #5

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

kamilkrzyskow
Copy link

@kamilkrzyskow kamilkrzyskow commented Nov 10, 2024

I fat fingered the enter key on title and it created an empty PR 🙄

Anyway, I always wanted to be able to access script data from Python, there is PyDecDat but it doesn't support Ikarus/Lego extended scripts, and iirc was rather slow.
So for now I typically used DecDat and parsed the .d file with Python to extract data.

When I saw this project I was happy that I don't have to parse the .d myself 😄...or so I thought.
Turned out there are some kinks to be straightened out, so I will fix them along the way.
Edits for maintainers are enabled, so if you want to adjust some things, then go ahead ✌️

My goal is not to create another DecDat, or so I think for now 🤔
My goal is to easily extract data, and cross-connected it afterwards in my external scripts.

Example of previously extracted data about dialogues of an NPC together with their possible routines:

"NONE_2246_BRADLOCK": {
  "dialogues": {
    "DIA_BRADLOCK_AFTERMINE": "",
    "DIA_BRADLOCK_AMBIENT": "How are you doing?",
    "DIA_BRADLOCK_CANTPASS": "",
    "Some": "Other"
  },
  "id": 2246,
  "instance_name": "NONE_2246_BRADLOCK",
  "name": "Bradlock",
  "routines": [
    "START",
    "Q308",
    "GUARD",
    "VOLKERTRIALOG",
    "TOT"
  ]
},

@kamilkrzyskow kamilkrzyskow changed the title Some fixes Some fixes and features to make it easier to access data Nov 10, 2024
Copy link
Member

@lmichaelis lmichaelis left a comment

Choose a reason for hiding this comment

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

Hi! Thank you very much for submitting a PR! I like the changes you're proposing, however I can see an issue with setting the string encoding. The way it is now, whenever zenkit._native.load is called with an encoding, it will change the encoding for all strings loaded by the library. This is unexpected behaviour, I'd think.

I propose instead, to add a function for setting the encoding globally and then applying ot to all string encoding and decoding performed by the library (e.g. a zenkit.set_string_encoding function). This should then also replace the hardcoded "windows-1252" in all str.encode calls (you don't need to do that right now, but that'd be the plan).

@lmichaelis lmichaelis added the enhancement New feature or request label Nov 10, 2024
src/zenkit/_native.py Outdated Show resolved Hide resolved
@kamilkrzyskow
Copy link
Author

kamilkrzyskow commented Nov 12, 2024

What's the "real" functional difference between DaedalusScript and DaedalusVm? The latter is a subclass, and the load function has a different value. I began working with DaedalusScript, but then trying to load DaedalusInstance.from_native resulted in INVALID type. Fortunately, the from_native is used in the code, so I could find that the symbol first needs to be initialized with DaedalusVm.init_instance. Would it be possible to move the init_instance to DaedalusScript?

Would it be possible to detect if a symbol is a function parameter inside the DLL, other than the .PAR suffix?
I was thinking about adding a is_param which would check the name, but perhaps there is something more performant 🤔

The DaedalusScript.symbols property generates a list of symbols every time it's accessed. This is a huge performance hog.
I'm not sure whether this should be cached (either with LRU / directly as self._symbols), or replaced with a generator to force the user to make their own list(symbols).

Same goes to other aspects, like the get_parent, I'm not sure whether or not to cache parent indexes for faster lookup in subsequent symbols 🤔

Lastly, it seems that DaedalusVm requires registering externals, even the defaults from Gothic. Why can't ZenKit provide them with the DLL, is there some licensing issue?

@lmichaelis
Copy link
Member

The difference between DaedalusVm and DaedalusScript is, that the script cannot run any Daedalus code. Its purpose is to facilitate access to all symbols in the binary as well as the bytecode instructions associated with them. The VM is capable of actually running code.

Daedalus instances are runtime-initialized objects, meaning they have to be explicitly initialized by the engine by running some Daedalus code. That's the job of the VM. So no, init_instance can't be moved into DaedalusScript. You can, however, create a VM the same way as a script, and as you already noticed, you will retain access to all functionality of DaedalusScript.


Getting the parameters of a Daedalus function isn't as simple as checking for a .PARn suffix. ZenKit has a function for getting the parameters of a given function, though I am not sure if it's exposed to Python right now. I am unsure if an is_param on symbols is feasible with the current DAT parser.


Caching is something I haven't considered yet, because there are many cases in ZenKit where caching is the wrong answer. In those examples specifically though, I think caving would be a valid strategy, because the script itself is immutable. A generator could be great too.


Externals are not registered by ZenKit, because most of them have something to do with game engine stuff, which ZenKit is not concerned with. Of course, things like CONCATSTRINGS could be implmented without issue, but WLD_InsertNpc is another story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants