Author Topic: [CODE REVIEW] Adding Beam Collision Hooks to Lua  (Read 1602 times)

0 Members and 1 Guest are viewing this topic.

Offline Axem

  • 211
[CODE REVIEW] Adding Beam Collision Hooks to Lua
Code: [Select]
Index: code/parse/lua.cpp
===================================================================
--- code/parse/lua.cpp (revision 10206)
+++ code/parse/lua.cpp (working copy)
@@ -14505,6 +14505,8 @@
    return ade_set_args(L, "o", l_Waypoint.Set(object_h(objp)));
   case OBJ_WEAPON:
    return ade_set_args(L, "o", l_Weapon.Set(object_h(objp)));
+  case OBJ_BEAM:
+   return ade_set_args(L, "o", l_Beam.Set(object_h(objp)));
   default:
    return ade_set_args(L, "o", l_Object.Set(object_h(objp)));
  }
Index: code/parse/scripting.cpp
===================================================================
--- code/parse/scripting.cpp (revision 10206)
+++ code/parse/scripting.cpp (working copy)
@@ -14,6 +14,7 @@
 #include "io/key.h"
 #include "controlconfig/controlsconfig.h"
 #include "freespace2/freespace.h"
+#include "weapon/beam.h"
 
 //tehe. Declare the main event
 script_state Script_system("FS2_Open Scripting");
@@ -75,7 +76,8 @@
  {"On Turret Fired",   CHA_ONTURRETFIRED, 0},
  {"On Primary Fire",   CHA_PRIMARYFIRE, 0},
  {"On Secondary Fire",  CHA_SECONDARYFIRE, 0},
- {"On Ship Arrive",   CHA_ONSHIPARRIVE, 0}
+ {"On Ship Arrive",   CHA_ONSHIPARRIVE, 0},
+ {"On Beam Collision",  CHA_COLLIDEBEAM, 0}
 };
 
 int Num_script_actions = sizeof(Script_actions)/sizeof(flag_def_list);
@@ -307,10 +309,12 @@
    case CHC_WEAPONCLASS:
     {
      if (!(action == CHA_ONWPSELECTED || action == CHA_ONWPDESELECTED || action == CHA_ONWPEQUIPPED || action == CHA_ONWPFIRED || action == CHA_ONTURRETFIRED )) {
-      if(objp == NULL || objp->type != OBJ_WEAPON)
+      if(objp == NULL || (objp->type != OBJ_WEAPON && objp->type != OBJ_BEAM))
        return false;
-      else if(stricmp(Weapon_info[Weapons[objp->instance].weapon_info_index].name, scp->data.name) != 0)
+      else if((stricmp(Weapon_info[Weapons[objp->instance].weapon_info_index].name, scp->data.name) != 0) && (objp->type == OBJ_WEAPON))
        return false;
+      else if((stricmp(Weapon_info[Beams[objp->instance].weapon_info_index].name, scp->data.name) != 0) && (objp->type == OBJ_BEAM))
+       return false;
      } else if(objp == NULL || objp->type != OBJ_SHIP) {
       return false;
      } else {
Index: code/parse/scripting.h
===================================================================
--- code/parse/scripting.h (revision 10206)
+++ code/parse/scripting.h (working copy)
@@ -70,6 +70,7 @@
 #define CHA_PRIMARYFIRE  28
 #define CHA_SECONDARYFIRE 29
 #define CHA_ONSHIPARRIVE 30
+#define CHA_COLLIDEBEAM  31
 
 // management stuff
 void scripting_state_init();
Index: code/weapon/beam.cpp
===================================================================
--- code/weapon/beam.cpp (revision 10206)
+++ code/weapon/beam.cpp (working copy)
@@ -35,8 +35,8 @@
 #include "iff_defs/iff_defs.h"
 #include "globalincs/globals.h"
 #include "cmdline/cmdline.h"
+#include "parse/scripting.h"
 
-
 extern int Cmdline_nohtl;
 // ------------------------------------------------------------------------------------------------
 // BEAM WEAPON DEFINES/VARS
@@ -2200,6 +2200,7 @@
 int beam_collide_ship(obj_pair *pair)
 {
  beam *b;
+ object *weapon_objp;
  object *ship_objp;
  ship *shipp;
  ship_info *sip;
@@ -2217,6 +2218,7 @@
  Assert(pair->a->instance >= 0);
  Assert(pair->a->type == OBJ_BEAM);
  Assert(Beams[pair->a->instance].objnum == OBJ_INDEX(pair->a));
+ weapon_objp = pair->a;
  b = &Beams[pair->a->instance];
 
  // Don't check collisions for warping out player if past stage 1.
@@ -2361,8 +2363,27 @@
  // if we got a hit
  if (valid_hit_occurred) {
   // add to the collision_list
-  beam_add_collision(b, ship_objp, &mc, quadrant_num);
 
+  Script_system.SetHookObjects(4, "Ship", ship_objp, "Beam", weapon_objp, "Self",ship_objp, "Object", weapon_objp);
+  bool ship_override = Script_system.IsConditionOverride(CHA_COLLIDEBEAM, ship_objp);
+
+  Script_system.SetHookObjects(2, "Self",weapon_objp, "Object", ship_objp);
+  bool weapon_override = Script_system.IsConditionOverride(CHA_COLLIDESHIP, weapon_objp);
+
+  if(!ship_override && !weapon_override) {
+     beam_add_collision(b, ship_objp, &mc, quadrant_num);
+  }
+
+  Script_system.SetHookObjects(2, "Self",ship_objp, "Object", weapon_objp);
+  if(!(weapon_override && !ship_override))
+   Script_system.RunCondition(CHA_COLLIDEBEAM, '\0', NULL, ship_objp);
+
+  Script_system.SetHookObjects(2, "Self",weapon_objp, "Object", ship_objp);
+  if((weapon_override && !ship_override) || (!weapon_override && !ship_override))
+   Script_system.RunCondition(CHA_COLLIDESHIP, '\0', NULL, weapon_objp);
+
+  Script_system.RemHookVars(4, "Ship", "Beam", "Self","Object");
+
   // if we got "tooled", add an exit hole too
   if (hull_exit_collision)
    beam_add_collision(b, ship_objp, &mc_hull_exit, quadrant_num, 1);
@@ -2439,7 +2460,30 @@
  // if we got a hit
  if(test_collide.num_hits){
   // add to the collision list
-  beam_add_collision(b, pair->b, &test_collide);
+
+  Script_system.SetHookObjects(4, "Beam", pair->a, "Asteroid", pair->b, "Self",pair->a, "Object", pair->b);
+  bool weapon_override = Script_system.IsConditionOverride(CHA_COLLIDEASTEROID, pair->a);
+
+  Script_system.SetHookObjects(2, "Self",pair->b, "Object", pair->a);
+  bool asteroid_override = Script_system.IsConditionOverride(CHA_COLLIDEBEAM, pair->b);
+
+  if(!weapon_override && !asteroid_override)
+  {
+   beam_add_collision(b, pair->b, &test_collide);
+  }
+
+  Script_system.SetHookObjects(2, "Self",pair->a, "Object", pair->b);
+  if(!(asteroid_override && !weapon_override))
+   Script_system.RunCondition(CHA_COLLIDEASTEROID, '\0', NULL, pair->a);
+
+  Script_system.SetHookObjects(2, "Self",pair->b, "Object", pair->a);
+  if((asteroid_override && !weapon_override) || (!asteroid_override && !weapon_override))
+   Script_system.RunCondition(CHA_COLLIDEBEAM, '\0', NULL, pair->b);
+
+  Script_system.RemHookVars(4, "Beam", "Asteroid", "Self","Object");
+  return 0;
+
+
  }
 
  // add this guy to the lighting list
@@ -2510,7 +2554,33 @@
  // if we got a hit
  if(test_collide.num_hits){
   // add to the collision list
+
+  Script_system.SetHookObjects(4, "Beam", pair->a, "Weapon", pair->b, "Self",pair->a, "Object", pair->b);
+  bool a_override = Script_system.IsConditionOverride(CHA_COLLIDEWEAPON, pair->a);

+  //Should be reversed
+  Script_system.SetHookObjects(2, "Self",pair->b, "Object", pair->a);
+  bool b_override = Script_system.IsConditionOverride(CHA_COLLIDEBEAM, pair->b);
+
+  if(!a_override && !b_override){
   beam_add_collision(b, pair->b, &test_collide);
+  }
+
+
+  if(!(b_override && !a_override))
+  {
+   Script_system.SetHookObjects(4, "Beam", pair->a, "Weapon", pair->b, "Self",pair->a, "Object", pair->b);
+   Script_system.RunCondition(CHA_COLLIDEWEAPON, '\0', NULL, pair->a);
+  }
+  if((b_override && !a_override) || (!b_override && !a_override))
+  {
+   //Should be reversed
+   Script_system.SetHookObjects(4, "Weapon", pair->b, "Beam", pair->a, "Self",pair->b, "Object", pair->a);
+   Script_system.RunCondition(CHA_COLLIDEBEAM, '\0', NULL, pair->b);
+  }
+
+  Script_system.RemHookVars(4, "Weapon", "Beam", "Self","Object");
+
  }
 
  // reset timestamp to timeout immediately
@@ -2576,8 +2646,29 @@
 
  // if we got a hit
  if(test_collide.num_hits){
-  // add to the collision list
-  beam_add_collision(b, pair->b, &test_collide);
+
+  Script_system.SetHookObjects(4, "Beam", pair->a, "Debris", pair->b, "Self", pair->a, "Object", pair->b);
+  bool weapon_override = Script_system.IsConditionOverride(CHA_COLLIDEDEBRIS, pair->a);
+
+  Script_system.SetHookObjects(2, "Self",pair->b, "Object",  pair->a);
+  bool debris_override = Script_system.IsConditionOverride(CHA_COLLIDEBEAM, pair->b);
+
+  if(!weapon_override && !debris_override)
+  {
+   // add to the collision list
+   beam_add_collision(b, pair->b, &test_collide);
+  }
+
+  Script_system.SetHookObjects(2, "Self", pair->a, "Object", pair->b);
+  if(!(debris_override && !weapon_override))
+   Script_system.RunCondition(CHA_COLLIDEDEBRIS, '\0', NULL, pair->a);
+
+  Script_system.SetHookObjects(2, "Self", pair->b, "Object", pair->a);
+  if((debris_override && !weapon_override) || (!debris_override && !weapon_override))
+   Script_system.RunCondition(CHA_COLLIDEBEAM, '\0', NULL, pair->b);
+
+  Script_system.RemHookVars(4, "Beam", "Debris", "Self","Object");
+
  }
 
  // add this guy to the lighting list

Because the beam collision code is sort of separate, the normal conditional collision hooks don't detect beam collisions. This is a patch that adds a $On Beam Collide: conditional hook, as well as the necessary code additions that allow the other $On * Collide Hooks to trigger with beam collisions. It's similar in structure to the other collision hooks, just instead of hv.Weapon, you have hv.Beam. I've done some simple tests and it seems to be working well. There's nothing earth shattering, but a second set of eyes is always handy. (Equal credit for this goes to Wanderer for making On Beam Collide and revising some of my silly copy pasta code)

 

Offline niffiwan

  • 211
  • Eluder Class
Re: [CODE REVIEW] Adding Beam Collision Hooks to Lua
OK, I've pushed this into a git branch to make it easier to pass the patch around :)
https://github.com/niffiwan/fs2open.github.com/commit/ea28597a6b9673cac5dbd452f4f05c8895067861

Now, for some comments:

1) I think that these lines should swap the order of the comparisons.

Code: [Select]
else if((stricmp(Weapon_info[Weapons[objp->instance].weapon_info_index].name, scp->data.name) != 0) && (objp->type == OBJ_WEAPON))
...
else if((stricmp(Weapon_info[Beams[objp->instance].weapon_info_index].name, scp->data.name) != 0) && (objp->type == OBJ_BEAM))

If the objp->type == comparison is done 1st then (by logical AND shortcut evaluation) there should be no chance of an invalid array index being passed into Weapons[] and Beams[]


2) I realise that everything in this point is basically copied from other existing collision hooks so presumably "it just works", but there's just a few points that I really don't get and that I was hoping someone could explain to me :)

Code: [Select]
+    Script_system.SetHookObjects(2, "Self",ship_objp, "Object", weapon_objp);
+    if(!(weapon_override && !ship_override))
+      Script_system.RunCondition(CHA_COLLIDEBEAM, '\0', NULL, ship_objp);
+
+    Script_system.SetHookObjects(2, "Self",weapon_objp, "Object", ship_objp);
+    if((weapon_override && !ship_override) || (!weapon_override && !ship_override))
+      Script_system.RunCondition(CHA_COLLIDESHIP, '\0', NULL, weapon_objp);

In the 1st set of statements, the only time the if will return FALSE is when weapon_override is TRUE & ship_override is FALSE.  But why do we want to skip running the "On Beam Collide" hook for any reason?  I would have thought that it should be called all the time, since override just means that FSO shouldn't handle the collision result and that scripting will do it.

In the 2nd set of statements, in addition to the issue I just mentioned, the if statement is exactly the same as:
Code: [Select]
if (!ship_override)

Is it really meant to be the inverse of the previous check?  Or something else?

(note: both these issues/questions apply to the other places the hooks were added)

I'm guessing that your tests would have been without override set on either hook, and that would be the most likely use-case so this is not that important. But I just can't wrap my head around what is trying to be achieved here re: not running hooks when "override-FSO" is set. :confused:
Creating a fs2_open.log | Red Alert Bug = Hex Edit | MediaVPs 2014: Bigger HUD gauges | 32bit libs for 64bit Ubuntu
----
Debian Packages (testing/unstable): Freespace2 | wxLauncher
----
m|m: I think I'm suffering from Stockholm syndrome. Bmpman is starting to make sense and it's actually written reasonably well...

 

Offline niffiwan

  • 211
  • Eluder Class
Re: [CODE REVIEW] Adding Beam Collision Hooks to Lua
and after some discussion today on IRC, committed in r10253.  1st fixes were added, and didn't worry about the 2nd set, all the other on collision hooks work, right?  (just don't use that override stuff unless you want risk Codethulu unleashed... :nervous:)
Creating a fs2_open.log | Red Alert Bug = Hex Edit | MediaVPs 2014: Bigger HUD gauges | 32bit libs for 64bit Ubuntu
----
Debian Packages (testing/unstable): Freespace2 | wxLauncher
----
m|m: I think I'm suffering from Stockholm syndrome. Bmpman is starting to make sense and it's actually written reasonably well...

 
Re: [CODE REVIEW] Adding Beam Collision Hooks to Lua
Wit a sec..
Does this imply star wars-esque light sabres?  :nervous: :eek2:

Or just the impact of the beam on ships?

 

Offline niffiwan

  • 211
  • Eluder Class
Re: [CODE REVIEW] Adding Beam Collision Hooks to Lua
sorry - no beam to beam collisions, just access to beam and "other object" collisions being provided to lua.
Creating a fs2_open.log | Red Alert Bug = Hex Edit | MediaVPs 2014: Bigger HUD gauges | 32bit libs for 64bit Ubuntu
----
Debian Packages (testing/unstable): Freespace2 | wxLauncher
----
m|m: I think I'm suffering from Stockholm syndrome. Bmpman is starting to make sense and it's actually written reasonably well...

  

Offline z64555

  • 210
  • Self-proclaimed controls expert
    • Minecraft
    • Steam
Re: [CODE REVIEW] Adding Beam Collision Hooks to Lua
"Don't cross the beams!" - Egon

I don't think the engine is ready to divert, morph, warp, etc. beams just yet.
Secure the Source, Contain the Code, Protect the Project
chief1983

------------
funtapaz: Hunchon University biologists prove mankind is evolving to new, higher form of life, known as Homopithecus Juche.
z64555: s/J/Do
BotenAlfred: <funtapaz> Hunchon University biologists prove mankind is evolving to new, higher form of life, known as Homopithecus Douche.