ryobg / JContainers

JSON-based data structures for Papyrus - the TESV Skyrim SE scripting language
MIT License
107 stars 23 forks source link

Port State Functions and project builds #1

Closed ryobg closed 6 years ago

ryobg commented 6 years ago

As it turns out original code did some custom changes to SKSE. Here I will keep a track of notes wrt to changes done in x64 branch in order to support State Functions. This currently is blocking issue to continue with the build.

Some insight was provided for that though comment #1 and comment #2 - thanks javierhimura!

javierhimura commented 6 years ago

I checked the files that are not the same between 1.7.0.1 and JContainers and all the changes from JContainers in PapyrusArgs.cpp and PapyrusArgs.h are included

I think that means the only files actually changed in JContainers are PapyrusNativeFunctionDef.inl and PapyrusNativeFunctionDef_Base.inl

I am not sure how to merge the part of LongSignature in PapyrusNativeFunctionDef.inl, in the original JContainers file only ShortSignature was included, LongSignature is a later adition from SKSE team, but at the same time probably JContainers only needs to have LongSignature adapted with state functions for compiling, it cant use a function that did not exits when the project was made.

javierhimura commented 6 years ago

I commit a merge with the changes, i cant test if the change compile until i arrive home, that is why i did not made a merge request yet

https://github.com/javierhimura/JContainers/commit/948aabc8b3a86a9bd03bd3ef7ef9ad2430fc7eab

ryobg commented 6 years ago

I just started to take a look at your changes.

https://github.com/javierhimura/JContainers/blob/948aabc8b3a86a9bd03bd3ef7ef9ad2430fc7eab/dep/skse/skse64/PapyrusNativeFunctionDef.inl#L28

I think here you should adopt the definition of the class which will be available later in the JContainer code. See the original code.

#define CLASS_NAME __MACRO_JOIN__(NativeFunctionWithState, NUM_PARAMS)

ryobg commented 6 years ago

Small other remark wrt to the added shit structure - I would check to change the type of the callback there depending on whether it is Long callback or not. Maybe in practice it matters not, but I can't check now.

Something to check for is whether the JContainer callback should be based on the long or short callback. I'm not sure what is the difference and if the long ones are to map into the higher addresses, the code can crash.

I will be unavailable in the next several hours. Thanks for all!

javierhimura commented 6 years ago

In the original SKSE code in JContainers only the definition of short callback was included, it seems long callback was added in SKSE releases after the last commit of JContainers

ryobg commented 6 years ago

I'm now onto it, will incorporate your logic and go further. Will post here what happened.

ryobg commented 6 years ago

Ok. I think that solution looks good enough. Testing will say. I suspect that there is a memory leak as it is not apparent who delete shit.

Currently there are unresolved linking errors, as it is not clear for me how the SKSE code should be linked. I have to check the original project.

javierhimura commented 6 years ago

For the linker in the source of the Oldrim version of SKSE inside the api_example project there is a file export.def, it does not exits in the source of SKSE64 because it does not include the source of the api example, but probably those plugins have been made based on the api_example and that means it is only needed to copy export.def to the JContainers project folder

javierhimura commented 6 years ago

I made tha commit that allow JContainers project to compile, i turn SKSE project into a static library with a reference to the new SKSE64.lib in JContainers, the post-build project still fails because the compilation is creating the dll in a folder x64/Release but the python code is searching in /Release folder, but at leas with this a dll can be compiled to test it in game https://github.com/javierhimura/JContainers/commit/ba91e03f7f7726650a5db62d25a40bd81b98e6c0 Probably this is not the best solution, with this change all the content of skse64.dll is being duplicated inside JContainers.dll

EDIT: In the original JContainers source code the SKSE project is also a Static Library https://github.com/SilverIce/JContainers/blob/develop/dep/skse/skse/skse.vcxproj

ryobg commented 6 years ago

Hey, I didn't had internet connection so went ahead. What you say I can confirm. I just pushed here and later will see how to fix the Post-Build project. So far everything links and we have dlls.

ryobg commented 6 years ago

Ok, a disitrubtion archive can be made in Release configuration. Ready for testing I think.