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

[Feature] Adding ini_file luabind constructor override. #1401

Merged

Conversation

Neloreck
Copy link
Contributor

Adding additional constructor override for ini_file luabind export class.
As result, modders will be able to create ini files placed not only in gamedata/configs folder.

In my case it may be useful for extensions feature, where separate extensions can be packed as script+ltx files without file system conflicts.

Changes:

  • Additional ini_file constructor override

@Neloreck Neloreck force-pushed the feature/different-folders-ini-files branch from 4781b9e to 966e008 Compare July 17, 2023 02:07
@Xottab-DUTY Xottab-DUTY added Enhancement Lua Modmaker Experience Modmaker experience with OpenXRay labels Jul 17, 2023
Copy link
Member

@Xottab-DUTY Xottab-DUTY left a comment

Choose a reason for hiding this comment

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

This looks similar feature to Anomaly, although, it's a bit different.
Could we make it the same as in Anomaly?

@Neloreck
Copy link
Contributor Author

@Xottab-DUTY
Checked anomaly variant recently

  1. For scripts they are using simple files list for-each and call on_game_start callback of every file if it is defined
  2. For ltx files anomaly modders are typically using dltx-modded exe (https://igigog.github.io/anomaly-modding-book/addons/dltx.html), which is not part of anomaly itself as I understand. It introduces new syntax for ini files (somewhere here https://github.com/themrdemonized/STALKER-Anomaly-modded-exes/blob/main/diff%20files%20for%20devs/dltx-bugfix.diff#L623)

In my variant I just simply add one more constructor for luabind ini file to open files not only from default configs folder.

Regarding monolith engine:

https://bitbucket.org/anomalymod/xray-monolith/src/d369db0ce75b8101c8d0fe1b2fa7a84b04910a35/src/xrServerEntities/script_ini_file.cpp#lines-20
https://bitbucket.org/anomalymod/xray-monolith/src/d369db0ce75b8101c8d0fe1b2fa7a84b04910a35/src/xrServerEntities/script_ini_file.h#lines-21

I believe similar approach is used here, but in anomaly variant override adds one optional parameter in the list end. If it is provided, it simply ignores first parameter with ternary operator.


What I would do as next steps:

  • Still leave this new override since it follows general pattern where we can provide path base like $game_data$, $scripts$ and not simply ignore it
  • I would think whether we need additional signature with same approach as anomaly engine is using. If it is needed, I can simply add it here
  • I would think whether we need DLTX as part of openxray. Generally it looks nice, but changing all the implementation of ini parser is not what I can do and it can cause major regression since we do not have any unit tests as part of ini implementation. Adding it as R&D task/future thing is nice to have

@Neloreck
Copy link
Contributor Author

There is also thing called DXML: https://github.com/themrdemonized/STALKER-Anomaly-modded-exes/blob/main/DXML.md
People are creating interceptors and patch XML files in some order.

Did anyone ever proposed DLTX / DXML integration?

@Xottab-DUTY
Copy link
Member

I would think whether we need additional signature with same approach as anomaly engine is using. If it is needed, I can simply add it here

It would be helpful, if we want to make life life easier for modders, that are used to Anomaly features. So, my initial suggestion had only a small scope of this: only adding function override like in Anomaly :)
I didn't even knew about DLTX/DXML! And that's awesome that you've mentioned it! I totally agree with creating an R&D task 💚

If it is needed, I can simply add it here

I fully support this, especially since we have #1373 :)

@Neloreck
Copy link
Contributor Author

I will add one more override to match anomaly as soon, as I have time then

src/xrServerEntities/script_ini_file.cpp Outdated Show resolved Hide resolved
src/xrServerEntities/script_ini_file.h Outdated Show resolved Hide resolved
@Xottab-DUTY
Copy link
Member

Xottab-DUTY commented Oct 18, 2023

Anomaly matching constructor will be merged as a part of the Call of Chernobyl code merge, so I reverted it here to help merging process a bit.

Thank you!

@Xottab-DUTY Xottab-DUTY merged commit be09c7d into OpenXRay:dev Oct 18, 2023
1 of 20 checks passed
@Neloreck Neloreck deleted the feature/different-folders-ini-files branch October 18, 2023 16:58
@Xottab-DUTY
Copy link
Member

Xottab-DUTY commented Nov 15, 2023

Call of Chernobyl support can be tracked in #1529.

DLTX and DXML support should be further tracked and discussed in #1373

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Lua Modmaker Experience Modmaker experience with OpenXRay
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants