could this be related to the status table which shows oposit teams ?

IIRC the oposit leader isn't filled until you see the opposite team (maybe the opposite leader, not sure)

Ok, well. I think I at least grasp the problem case you describe, why the original algorithm causes it and yours doesn't. The VC debugger goes through your code without complaints which generally means you ain't doing bad stuff with iterators. I find it easier to understand with your modifications since you divide it more apart into parts. And yes I think you understand it probably better than me and I feel comfortable about you becoming a dev, since I think that you know what you're doing (that is the important question always).
So, I'd say the patch is save to apply.

And btw; this team::seen_ variable an related functions seem unused (you seem to have put a comment about it). I know of no such functionality (teams knowing about which other teams they know - all teams always know all other teams).

brilliand:

I thought about adding a "movethrough" event myself, but decided to fix the algorithm before adding complexities to it. On the other hand, if adding complexities fixes the algorithm as a side-effect, that might be a more efficient course of action. Let me see what you have done...

I see your patch takes the "trailing pointer" approach. I had considered that, but I found it insufficient in at least one case. Since I was going to end up essentially going through the route twice anyway, I opted for an approach I consider easier to follow when reading the code.

Your patch appears to not handle that one case correctly, and I think there are some other oddball cases that it also falls short on. I'll run some tests to confirm, and if it does fall short, I can give you some test scenarios to work with. Or we can leave it as a question of which is more pressing -- adding your "movethrough" event or making movement work correctly in obscure cases.

So what is this case that made me drop the trailing pointer approach? It can get complicated in "typical" situations just because of all the factors that can be involved, so I'll push it to an extreme, yet simple, version. Suppose you are in a scenario with fog, and your quick lancer is in the middle of flat terrain surrounded by friendly heavy infantrymen. Really surrounded, as in every hex the lancer could move to this turn is occupied. If you click the lancer, then click an unoccupied hex (setting up a multi-turn move), there should be no effect because of the infantry. However, if you move the infantryman who is where the lancer wanted to end this turn, then repeat clicking the lancer and the multi-turn destination hex, the lancer should now move to the just-vacated hex, and clear fog from every hex along the first leg of that multi-turn route.

Here is the problem: how do you know which hexes to clear of fog before you know if you have a hex you can move to? (If you clear fog before determining if the lancer can actually move, you allow the lancer to scout in all directions before committing to any actual movement.) My approach is to first see where the lancer can go based on known information, then iterate through the route a second time to handle discoveries, such as clearing fog.

Sure, this happens all the time, Linux-devs breaking compilation on other systems. And vice versa, but me no longer since I use a recent MinGw compiler to countercheck (which didn't complain here.)

Yes, that works.

movement-split.diff doesn't compile in MSVC,
in the

// Fire sighted event here
for (sight_it = stops.seen_units.begin();
sight_it != stops.seen_units.end(); ++sight_it)

part it can't make the sight_it = stops.seen_units.begin() assignment due to missing operator (error C2679); movement-split-improved.diff improved however compiles and this is supposedly what you want to get applied.

Smaller functions? I could do that. (In fact, I would prefer that. It just seemed as though the existing style had a preference for monolithic functions, so I went with what was there.)

Yes, same name from the forums.

Yes, this is "dangerous" in the sense you mean. Then again, the timing for such a change seems good as a new development cycle has just begun, so there is about as much time as possible for people to find any bugs before it gets released. (Not that I think there are any bugs, just agreement that such changes have to be assumed to be likely to introduce some.) Plus, I do believe that making the foundation more robust can reduce the number of bugs introduced in the future (so this would sort of be an investment in future growth for Wesnoth).

Well, and I guess I went for something like this because I know just how good I am, even though there is no reason for anyone in the project to know that (nor to believe me just because I said so). I do tend to go for the harder problems, and scepticism of what I offer is understandable. I just ask for a chance to be considered "not a crackpot".

I was looking through some of Wesnoth's code when I noticed some errors in the algorithm used for calculating movement. Nothing major, though, as the errors only occur in somewhat obscure cases. I thought it might be easier to fix up the code than to document all the oddball cases, so here is a patch with a corrected algorithm for move_unit() in actions.cpp.

So that I am not asking for blind trust that problems exist, I did take screenshots of one of the least obscure problem cases. The attached images show the "before" and "after" of a mouse click. The salient points are the following.
1) The unit's planned movement for the current turn ends in an enemy's ZoC.
2) The unit's planned movement for the current turn ends on a friendly unit.
3) The unit did actually move after clicking.
I would expect the unit's remaining movement to be decreased by the cost of the hexes it entered (so decreased to 8 in the example case). Instead, movement is set to the cost to enter the hexes it planned to enter this turn but did not (so was set to 1 for the second image).