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] Extending upgrade api for scripting, correct some upgrading logics #1463

Merged
merged 4 commits into from
Oct 18, 2023

Conversation

Neloreck
Copy link
Contributor

Changes

  • Unified net_Spawn_install_upgrades logics to work with same signature as original net_Spawn event and expect abstract entity
  • Moved net_Spawn_install_upgrades logics call before base method related to net spawning and binder lifecycle calls [A]
  • Less strict check in can_install when loading data -> does not make sense to check data when loading from file, it should be validated on moment of addition [B]
  • Added/exported method to check if upgrade group is installed by upgrade id (has_upgrade_group_by_upgrade_id)
  • Added/exported method to check if upgrade group is installed
  • Added/exported method to check if upgrade can be added [C]
  • Added/exported method to check if upgrade can be installed [C]

A

Previous implementation of net_Spawn_install_upgrades called cleanup of client side object m_upgrades and tried to sync it with server entity. Also it was called after binder lifecycle methods from script so it made all the logics used in scripts on spawn event irrelevant (LUA added extension -> it is added to client object -> it is added to server object -> inventory item lifecycle deletes all the data from client object and fetches it from temporary spawn entity where it is empty).

Also I believe some code parts mocked this method and called original one before base lifecycle specifically for same reasons.

B

On load event we should not re-validate everything what was done before by game engine. Also keeping old logics blocks us from installing all upgrades with scripts. Once you are loading save where 2 upgrades from same group are installed, game crashes without these changes. So basically the change solves two problems.

C

Added new methods and separated concept of install and add for compatibility and code quality. Problem is that install runs script checks and script effects which are part of interaction with mechanic. If you want to purely add upgrades based only on script logics you have to use something else.

add just adds and saves upgrade ignoring script effects/preconditions and it does not unload/detach addons from weapons and armor.

Usage

As result, I was able to create logics in my mod where all the weapon/armor get random upgrades on spawn with random chance.
It makes looting more fun + there are situations with unique items where both upgrades from same group are activated.

@Neloreck Neloreck changed the title [FEATURE] Extending upgrade api for scripting, correct some upgrading logics [Feature] Extending upgrade api for scripting, correct some upgrading logics Oct 16, 2023
@Xottab-DUTY Xottab-DUTY added Enhancement Lua Modmaker Experience Modmaker experience with OpenXRay labels Oct 18, 2023
@Xottab-DUTY
Copy link
Member

@yohjimane, this might be interesting to you!

(could you also look at it/maybe review?)

src/xrGame/script_game_object_inventory_owner.cpp Outdated Show resolved Hide resolved
src/xrGame/script_game_object_inventory_owner.cpp Outdated Show resolved Hide resolved
src/xrGame/inventory_item_upgrade.cpp Outdated Show resolved Hide resolved
src/xrGame/ExplosiveRocket.cpp Outdated Show resolved Hide resolved
@yohjimane
Copy link
Contributor

@yohjimane, this might be interesting to you!

(could you also look at it/maybe review?)

Thanks for the heads up! Changes look good to me (e.g. shouldnt conflict with OXR gunslinger) other than some minor code style issues

@Neloreck Neloreck requested a review from yohjimane October 18, 2023 18:32
Copy link
Contributor

@yohjimane yohjimane left a comment

Choose a reason for hiding this comment

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

Looks good, thank you Neloreck :)

src/xrGame/inventory_item_upgrade.cpp Outdated Show resolved Hide resolved
src/xrGame/inventory_item_upgrade.cpp Outdated Show resolved Hide resolved
src/xrGame/script_game_object_inventory_owner.cpp Outdated Show resolved Hide resolved
src/xrGame/script_game_object_inventory_owner.cpp Outdated Show resolved Hide resolved
@Neloreck Neloreck force-pushed the feature/upgrading-extension branch from 6582e52 to c239d9e Compare October 18, 2023 20:51
@Neloreck
Copy link
Contributor Author

Rebased / updated upgrades iteration

@Xottab-DUTY
Copy link
Member

Xottab-DUTY commented Oct 18, 2023

Thanks! Waiting for CI to finish, and merging!

@Xottab-DUTY Xottab-DUTY merged commit a6016f1 into OpenXRay:dev Oct 18, 2023
19 of 20 checks passed
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.

3 participants