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

Fix wrapping of toplevel gadget callins returning several parameters. #3947

Merged

Conversation

saurtron
Copy link
Collaborator

@saurtron saurtron commented Nov 20, 2024

Work done

  • Fix wrapping of top level callins returning several parameters.
  • Affected methods: GameSetup, AllowWeaponTarget, UnitPreDamaged, FeaturePreDamaged

Explanation

Likely affected gadgets:

  • GameSetup: none, still it can be affecting some engine calls since it will always return newReady nil, although afaics the engine won't accept the nil value so it shouldn't be affecting it.
  • AllowWeaponTarget (affects returned priority): unit_aa_targeting_priority.
  • UnitPreDamaged (affects impulse only): unit_combomb_full_damage, unit_dgun_behaviour, engine_hotfixes, unit_paralyze_damage_multiplier, map_lava, unit_paralyze_on_off, unit_collision_damage_behavior, unit_timeslow, unit_objectify, unit_evolution, unit_commando_watch, unit_shield_behaviour, unit_single_damage_fire, unit_lightning_splash_dmg, mo_preventcombomb, scav_spawner_defense, unit_no_land_damage.
  • FeaturePreDamaged (affects impulse only): gfx_tree_feller.

luarules/gadgets.lua Outdated Show resolved Hide resolved
luarules/gadgets.lua Outdated Show resolved Hide resolved
@sprunk
Copy link
Collaborator

sprunk commented Nov 20, 2024

Looks good.

The multi return branch could probably be used for everything (i.e. the patch would be a 2-liner that changes res to res1, res2 where it is currently used) and then we would avoid the perf cost of handling the multiReturnTopCallins table in each GC cycle, I guess it's imperceptible either way though.

@saurtron
Copy link
Collaborator Author

Looks good.

The multi return branch could probably be used for everything (i.e. the patch would be a 2-liner that changes res to res1, res2 where it is currently used) and then we would avoid the perf cost of handling the multiReturnTopCallins table in each GC cycle, I guess it's imperceptible either way though.

ok did that now it will wrap two return parameters for everything

@saurtron saurtron changed the title Fix toplevel gadget callins returning several parameters. Fix wrapping of toplevel gadget callins returning several parameters. Nov 20, 2024
@WatchTheFort WatchTheFort merged commit ac209d4 into beyond-all-reason:master Nov 21, 2024
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.

3 participants