lzrtag / LZRTag

AVR-Based easily modifyable DIY Lasertag!
GNU General Public License v3.0
94 stars 11 forks source link

Improved hit and firing handling options! #23

Closed Xasin closed 6 years ago

Xasin commented 6 years ago

This new PR, made to close #13 and close #14, phases out the old NoAmmoShoot.lua file, and replaces it with a shiny new one that gives the server full control over shooting speed, flashing, vibrations, and all other sorts of things.

To be precise:


This change is Reviewable

Xasin commented 6 years ago

Reviewed 3 of 8 files at r2, 1 of 4 files at r3, 4 of 6 files at r5, 4 of 4 files at r6. Review status: all files reviewed at latest revision, 9 unresolved discussions.


ESP/FireHandling.lua, line 99 at r6 (raw file):

  end

  shotTimer:start();

shotTimer:start() does not make sense if we re-register the alarm later anyways!


ESP/FireHandling.lua, line 109 at r6 (raw file):

      reloadTimer:alarm(fireConf.reloadDelay, tmr.ALARM_SINGLE,
          function()
              reloadAmmo();

Just pass the reloadAmmo() function directly, instead of defining a function to another function call.


ESP/FireHandling.lua, line 118 at r6 (raw file):

      if((data:byte(1) == 0) == invertButton) then
          eP = '{"type":"button","player":"' .. playerID .. '"}';
          homeQTT:publish(lasertagTopic .. "/Game/Events", eP, 0, 0);

Maybe add a Player/ShotLock or a config variable option to switch between button press reporting and auto-shooting?


ESP/ServerConfig.lua, line 10 at r6 (raw file):

  dead = false,
  ammo = 0,
  lastShot = 0,

Is this variable still needed?


ESP/ServerConfig.lua, line 64 at r6 (raw file):

subscribeTo(playerTopic .. "/HitConf", 1,
  function(data)
      if(data == "") then

Use string length detection here, instead of ""


ESP/ServerConfig.lua, line 87 at r6 (raw file):

      data = tonumber(data)
      if(data) then
          player.ammo = data;

This needs to invoke an ammo update.


Ruby/GameInterface/LTagPlayer.rb, line 73 at r6 (raw file):

  end
  def dead=(d)
      @dead = (d == true)

Have this be a proper to bool sort of transformation, please!


Ruby/GameInterface/LTagPlayer.rb, line 78 at r6 (raw file):


  def ammo=(a)
      raise ArgumentError, "Ammo amount needs to be a number!" unless a.is_a? Integer

Check for positivity here.


Ruby/GameInterface/LTagPlayer.rb, line 122 at r6 (raw file):

      @mqtt.publish_to "#{@mqttTopic}/Dead", "", retain: true;

      self.hitConfig = nil;

Fire Config also needs to be set to nil!


Comments from Reviewable

Xasin commented 6 years ago

Review status: all files reviewed at latest revision, 2 unresolved discussions.


ESP/FireHandling.lua, line 99 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
shotTimer:start() does not make sense if we re-register the alarm later anyways!

Done.


ESP/FireHandling.lua, line 109 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Just pass the reloadAmmo() function directly, instead of defining a function to another function call.

Done, in both parts of the code.


ESP/ServerConfig.lua, line 10 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Is this variable still needed?

Nope! Was removed.


ESP/ServerConfig.lua, line 64 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Use string length detection here, instead of ""

Done! It should now do this fairly well.


ESP/ServerConfig.lua, line 87 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
This needs to invoke an ammo update.

Ammo update AND attemptShot() both added


Ruby/GameInterface/LTagPlayer.rb, line 73 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Have this be a proper `to bool` sort of transformation, please!

Meh ... None there. Used the ? operator instead.


Ruby/GameInterface/LTagPlayer.rb, line 78 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Check for positivity here.

Doot doot done.


Comments from Reviewable

Xasin commented 6 years ago

Reviewed 2 of 3 files at r7, 1 of 1 files at r8. Review status: all files reviewed at latest revision, 1 unresolved discussion.


Ruby/GameInterface/LTagPlayer.rb, line 122 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Fire Config also needs to be set to nil!

Don huan :3


Comments from Reviewable

Xasin commented 6 years ago

Reviewed 2 of 2 files at r9. Review status: all files reviewed at latest revision, all discussions resolved.


ESP/FireHandling.lua, line 118 at r6 (raw file):

Previously, Xasin (Xasin) wrote…
Maybe add a `Player/ShotLock` or a config variable option to switch between button press reporting and auto-shooting?

Done! Doubled up actually; used fireConf and added both reportButton and shotLocked to give the server a little more control :D


Comments from Reviewable

Xasin commented 6 years ago

Codacy is being buggy with PR status reporting. It went through without problems, no status was created.