Xasin
commented
7 years ago Reviewed 8 of 15 files at r1, 2 of 3 files at r2, 1 of 2 files at r3, 6 of 9 files at r4, 3 of 3 files at r5, 1 of 1 files at r6. Review status: all files reviewed at latest revision, 11 unresolved discussions.
EclipseWS/MainBoard/main.cpp, line 57 at r5 (raw file):
Board::Nozzle::flash(0b10 << Board::Vest::team); Board::Buzzer::sweep(3000, 1000, 150); IR::TX::startTX({playerID, 0});
This needs changing for the Flexibility Milestone!
EclipseWS/MainBoard/Localcode/Board/VestBlink.cpp, line 90 at r5 (raw file):
case 5: if((phase%2 == 0) && (phase < 6)) {
This could also be replaced with a binary comparison
ESP/init.lua, line 4 at r5 (raw file):
dofile("PlayerData.lua"); lasertagTopic = "Lasertag"; playersTopic = lasertagTopic .. "/Players";
Maybe this can be phased out? Should work, right?
ESP/Lasertag.lua, line 4 at r5 (raw file):
dofile("UARTReceive.lua"); playerIDList = {}
This definitely turned unnecessary with the latest ID changes!
ESP/Lasertag.lua, line 69 at r5 (raw file):
); tmr.create():alarm(500, tmr.ALARM_SINGLE, function()
Why not publish this immediately?
ESP/MQTTConnect.lua, line 27 at r5 (raw file):
function(client) for k,v in pairs(sublist) do homeQTT:subscribe(k, v.qos);
This needs to be changed to a slow-subscription system, similar to the one used in MQTTTools Maybe there's a possibility for unification here?
ESP/MQTTTools.lua, line 58 at r5 (raw file):
if(not running) then subTimer:start(); end
As seen above - this could be split up into a separate "raw" function to integrate and start the slow-sub process, and apply it to the reconnects above.
Ruby/LTagGame.rb, line 2 at r5 (raw file):
require_relative 'MQTTSubscriber.rb'
Mhh ... Maybe change the position of MQTTSubscriber to a more "unified" library folder one? Possibly even integrate it into the PATH variable or something.
Ruby/LTagGame.rb, line 20 at r5 (raw file):
@clientUnregisteredCBs = Array.new(); @mqtt.subscribeTo "#{@mqttTopic}/Team" do |tList, data|
Mmmmeeehhh
Change those to the new subscribe_to
Just so the code at least looks sane
Ruby/LTagPlayer.rb, line 47 at r6 (raw file):
def brightness=(n) n = n.to_i; return false unless n != nil and n <= 5 and n >= 0;
Maybe do the same as below and raise errors, instead of simply returning a false.
Ruby/LTagPlayer.rb, line 84 at r6 (raw file):
console("overrideVest(0, 0);"); end
These should definitely be properly made data channels, possibly via a mix of various MQTT topics and a pack-ed array. We wanna be slick and fast, after all.
Comments from Reviewable
Basically ... Everything required to have an actually functional game. Weapon control via MQTT, ID assignment, and a Ruby backend.
Yay <3
This change is