calmackenzie / VG1819

Final Project for CS4830 2018 - 2019
0 stars 2 forks source link

Game desync'd after armory auto-used arm #423

Closed calmackenzie closed 5 years ago

calmackenzie commented 5 years ago

^ logs indicate that the receiving client is also sending it again, probably just have to do another check against client ID

calmackenzie commented 5 years ago

Experienced this issue after merge, reopening Not sure if this was a result of armory auto-arm, but desync happened after armory used auto-arm

kamarapala commented 5 years ago

Do you have more details for this? It would be helpful to know what units were before and after the Armory. I haven't been able to desync

kamarapala commented 5 years ago

I think I've figured it out.

I think recently, Unit had an update() loop added which calls UnitTurn::checkTurn, which will end the unit's turn if it can't move and can't act.

When Armory auto uses Arm, it won't end it's turn until the next frame. But, since the auto use happens so fast, there is the possibility that another action from the next unit could be received and attempted before the next frame ends the turn of the Armory. Ex.

This would explain the inconsistent reproduction of the issue.

kamarapala commented 5 years ago

I checked to make sure and it is indeed receiving multiple packets within the same frame whenever it desyncs, which makes sense as TCP doesn't guarantee data to be grouped separately.

@SRJYC, is it possible/difficult to have the units checkTurn() the same frame that they use an ability? I tried putting checkTurn() inside of actDone() but it seems due to some flags, if there are multiple Armories from the same Commander that are in sequence, then only the first Armory will autocast, and the rest must be manually casted.

SRJYC commented 5 years ago

emmm,checkTurn() used to be in actDone() and moveDone(), but auto cast is too fast, so Unit Interaction Manager will receive two requests at same frame. It will ignore requests that after first one. So, I put checkTurn() in Update(), so unit can end turn in different frame to prevent such thing happens.

1st Armory auto uses Arm, ends turn To break down this, the process should be:

  1. 1st Armory auto uses Arm, send request to unit interaction manager
  2. unit interaction manager trigger event to ask input
  3. Board manager auto click and trigger event to send info back

This is trigger by event system, from now on, they are method calls. They are all on the call stack.

  1. unit interaction manager calls ability manager to use ability
  2. Arm effect()
  3. 1st Armory's actDone() get called, unit::m_unit->m_act should turn to false at this step.
  4. At next frame, 1st Armory then will checkTurn() and end turn.

if move checkTurn back to actDone(), it will be

  1. 1st Armory checkTurn() and end turn
  2. Next unit start turn.
  3. If it has auto cast ability, it will send request to unit interaction manager
  4. but at this time unit interaction manager is still in call stack, which means last ability doesn't resolve completely. It will ignore the request, due to "busy" flag

If change "busy" flag before actually using ability, it may cause other problems. I think I tried it before, it doesn't work, but I forget what problem it causes.

SRJYC commented 5 years ago

P2: Receives 1st Armory Arm P2: Receives 2nd Armory Arm, before next frame

Maybe build a queue for this?

Or just remove auto cast or auto click?

kamarapala commented 5 years ago

I was thinking of a queue but briefly thinking about it, it could create some new issues.

I think for now the best solution would be to remove autocast from Armory and revisit this issue if I have time after helping with the other issues.

Although it's probably better to have autocast disabled as if there are more than 1 or 2 Armories, they all use Arm so quickly that it's difficult for the player to realize what happened; if we want autocast with autoclick, I think we'd have to add a wait for some delay or animation to finish before ending turn so players have a better visual of what's going on.

austinbrianrogers commented 5 years ago

I think removing auto-cast all together is worth it. I think it interrupts gameflow in a strange way.

SRJYC commented 5 years ago

I will disable the stuffs very quick.