[Widelands-cvs] SF.net SVN: widelands:[4594] trunk/src
Status: Beta
Brought to you by:
sirver
From: <si...@us...> - 2009-09-30 21:50:38
|
Revision: 4594 http://widelands.svn.sourceforge.net/widelands/?rev=4594&view=rev Author: sigra Date: 2009-09-30 21:50:28 +0000 (Wed, 30 Sep 2009) Log Message: ----------- Add some debug output to hunt down #2868683. Here is what might happen. The worker 3755 walks along the road 6330. He has a task transfer. On top of that, he has a task movepath, with the path of the road. Bob::movepath_update calls start_task_move (from (49, 143) in direction 4 (sw) towards (49, 144)), which calls push_task, which schedules an act after 10ms and then returns. Then the road is split by a flag that is placed at (49, 143). The road sees that the worker is on a location that is still on the path of the shrunken road ((49, 141), (50, 142), (49, 143)), so the worker is not reassigned to the new road. The road sends the signal 'road' to the worker. When the 10ms has passed, the worker acts. Bob::move_update is called. It sees that state.diranims is true, so it calls start_walk, which updates the worker's position to the next node (49, 144). start_walk returns the time that it takes to walk the edge. move_update uses that duration to scheduale a new act. When the worker acts again, it sees that the walkend time is <= the gametime and knows that it is done. It calls pop_task and returns. pop_task schedules and act after 10 ms. When the 10 ms has passed, the worker acts. Bob::movepath_update is called. It sees that the worker has a signal and therefore calls pop_task. pop_task schedules and act after 10 ms. When the 10 ms has passed, the worker acts. Worker::transfer_update is called. It sees that the signal is 'road' or 'fail' and ignores it. It sees that the worker's location is not a Building, not a Flag, but a Road. This is the shrunken road that the worker is no longer on!. The function calls start_task_movepath on the worker with the road's path. It fails because the worker is no longer on that path. The reason that this bug does not happen more frequently is that the road must be split during the 10ms after the move task has been pushed and before the worker acts and calls move_update, which calls start_walk, which updates the worker's position. If the road is split after that, the split code will see that the worker is on the second part of the road, and reassign it. So how should this bug be fixed? Would it work to remove that 10ms time window, by calling schedule_act with 0 delay? There seems to be several such short arbitrarily sized time windows in various places. After seeing this bug, it seems like a good idea to get rid of them. Is anyone aware of any proble with 0 delays in the command queue? Is it so, that if the act is scheduled with 0 delay, a playercommand, such as a road split, can not come between? Modified Paths: -------------- trunk/src/economy/road.cc trunk/src/logic/bob.cc trunk/src/logic/worker.cc Modified: trunk/src/economy/road.cc =================================================================== --- trunk/src/economy/road.cc 2009-09-30 18:31:29 UTC (rev 4593) +++ trunk/src/economy/road.cc 2009-09-30 21:50:28 UTC (rev 4594) @@ -407,6 +407,13 @@ path.truncate(index); secondpath.starttrim(index); + molog("splitting road: first part:\n"); + container_iterate_const(std::vector<Coords>, path.get_coords(), i) + molog("* (%i, %i)\n", i.current->x, i.current->y); + molog(" second part:\n"); + container_iterate_const(std::vector<Coords>, secondpath.get_coords(), i) + molog("* (%i, %i)\n", i.current->x, i.current->y); + // change road size and reattach m_flags[FlagEnd] = &flag; _set_path(game, path); @@ -453,7 +460,8 @@ } } - molog("Split: check %u -> idx %i\n", w.serial(), idx); + if (w.serial() == 3755) + molog("Split: check %u -> idx %i\n", w.serial(), idx); if (idx < 0) { reassigned_workers.push_back(&w); @@ -466,6 +474,12 @@ } } + if (w.serial() == 3755) { + molog + ("Split: sending signal 'road' to worker %u (at (%i, %i)):\n", + w.serial(), w.get_position().x, w.get_position().y); + w.log_general_info(game); + } // Cause a worker update in any case w.send_signal(game, "road"); } Modified: trunk/src/logic/bob.cc =================================================================== --- trunk/src/logic/bob.cc 2009-09-30 18:31:29 UTC (rev 4593) +++ trunk/src/logic/bob.cc 2009-09-30 21:50:28 UTC (rev 4594) @@ -34,6 +34,8 @@ #include "upcast.h" #include "wexception.h" +#include "backtrace.h" + namespace Widelands { /** @@ -655,9 +657,17 @@ CoordPath path(game.map(), origpath); int32_t const curidx = path.get_index(get_position()); - if (curidx == -1) + if (curidx == -1) { + molog + ("ERROR: (%i, %i) is not on the given path:\n", + get_position().x, get_position().y); + container_iterate_const(std::vector<Coords>, path.get_coords(), i) + molog("* (%i, %i)\n", i.current->x, i.current->y); + log_general_info(game); + log("%s", get_backtrace().c_str()); throw wexception ("MO(%u): start_task_movepath(index): not on path", serial()); + } if (curidx != index) { if (curidx < index) { @@ -678,8 +688,13 @@ void Bob::movepath_update(Game & game, State & state) { - if (get_signal().size()) + if (get_signal().size()) { + if (serial() == 3755) + molog + ("[movepath_update] signal '%s'; popping task\n", + get_signal().c_str()); return pop_task(game); + } assert(state.ivar1 >= 0); Path const * const path = state.path; @@ -755,6 +770,9 @@ void Bob::move_update(Game & game, State & state) { if (state.diranims) { + if (get_signal() == "road") + molog("ERROR!!!!!!!!!!!!!!!! GOT SIGNAL 'road' BEFORE start_walk\n"); + int32_t const tdelta = start_walk (game, @@ -768,14 +786,13 @@ return pop_task(game); } else return schedule_act(game, tdelta); + } else if (m_walkend <= game.get_gametime()) { + end_walk(); + return pop_task(game); } else - if (game.get_gametime() - m_walkend >= 0) { - end_walk(); - return pop_task(game); - } else - // Only end the task once we've actually completed the step - // Ignore signals until then - return schedule_act(game, m_walkend - game.get_gametime()); + // Only end the task once we've actually completed the step + // Ignore signals until then + return schedule_act(game, m_walkend - game.get_gametime()); } @@ -886,22 +903,22 @@ int32_t Bob::start_walk (Game & game, WalkingDir const dir, uint32_t const a, bool const force) { - FCoords newf; + FCoords newnode; Map & map = game.map(); - map.get_neighbour(m_position, dir, &newf); + map.get_neighbour(m_position, dir, &newnode); // Move capability check if (!force) { CheckStepDefault cstep(descr().movecaps()); - if (!cstep.allowed(map, m_position, newf, dir, CheckStep::stepNormal)) + if (!cstep.allowed(map, m_position, newnode, dir, CheckStep::stepNormal)) return -1; } // Always call checkNodeBlocked, because it might communicate with other // bobs (as is the case for soldiers on the battlefield). - if (checkNodeBlocked(game, newf, true) and !force) + if (checkNodeBlocked(game, newnode, true) and !force) return -2; // Move is go @@ -912,7 +929,11 @@ m_walkstart = game.get_gametime(); m_walkend = m_walkstart + tdelta; - set_position(game, newf); + if (serial() == 3755) + molog + ("[start_walk]: changint position from (%i, %i) to (%i, %i)\n", + get_position().x, get_position().y, newnode.x, newnode.y); + set_position(game, newnode); set_animation(game, a); return tdelta; // yep, we were successful Modified: trunk/src/logic/worker.cc =================================================================== --- trunk/src/logic/worker.cc 2009-09-30 18:31:29 UTC (rev 4593) +++ trunk/src/logic/worker.cc 2009-09-30 21:50:28 UTC (rev 4594) @@ -1209,7 +1209,7 @@ } // Signal handling - std::string signal = get_signal(); + std::string const signal = get_signal(); if (signal.size()) { // The caller requested a route update, or the previously calulcated route @@ -1307,6 +1307,10 @@ if (nextstep != &road.get_flag(Road::FlagEnd)) path.reverse(); + molog + ("[transfer]: starting task [movepath] and setting location to " + "road %u\n", + road.serial()); start_task_movepath (game, path, descr().get_right_walk_anims(does_carry_ware())); set_location(&road); This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. |