12051 Movement Trail option to only keep last position per location#14342
12051 Movement Trail option to only keep last position per location#14342rlament wants to merge 4 commits intovassalengine:release-3.7from
Conversation
Add an option to keep the last movement trail position only when location name or mat change. All off-board movement is always appended to the movement trail. Check for mat to mat movement within a location. Add unit tests for movement trails. Update movement trails documentation adding the new option.
|
@rlament Ray, you made the same mistake as I did. Step 1: I move some unit from area1 to area2 and then to area3 (green one, extreme left) Step 2: I move this unit inside area3 
Point in area2 is deleted too unfortunately. I will look into my code as I found a solution to this problem. To be honest when I was finishing this feature my mind was cooked because of such details ;-) |
|
@rlament However I agree that your solution seems simpler and more elegant and I think we should go with it. Just this one problem described above to fix. I will look into it now. |
|
Also if I move inside area2 the trail dissapears entirely. Step 1: Step 2: |
|
@rlament Ray, I think I more or less remember my problem. We want to compare locationName for actual position (place where you moved your piece) with the locationName of the last position on the trail. We wrote similar code. I think it was like that (I don't remember everything precisely).
I can't compare in PieceMover because I don't have Point p yet. I agree it's not elegant but seemed necessary. Now I lean again to my solution but what I understand from your code I didn't take into account that off board moves should always be recorded by movement trails, right? Please tell me what you think. Some data from debug after moving a unit to another area: PieceMover FootPrint |
|
@rlament Regarding "off board moves should always be recorded by movement trails" - how can I reproduce that? Can I do this in Turning Point Stalingrad or should I use another mod? |
@Niedowidek Can be done with that module. Zoom out all the way and drag a piece off below or to the side of the map. |
|
@rlament Does Volga River qualify as off-map ? I moved my piece several times along the river and only one position registers as in a separate area. Should every position on the trail along the Volga be visible ? That's what you have in mind? Also when I put my piece outside the map, the trail is only visible when still on the map, after passing map's edge it dissappears, as shown below. 
|
Yes, that looks correct. Except for the square units in the river, there are no zones defined for the river area. Anything moving there is labelled offboard since there is no zone. And yes, every position on the trail in the Volga should be visible. Should test for both offboard and off-map. Moving off-map the trail stops at the map boundary. In my testing I'll move off-map, shift to another location off-map and then move back on the map. The test is to verify that multiple consecutive offboard or off-map points are not removed from the movement trail. |
Align variable naming with MovementMarkable Expand unit tests
|
Without any code changes, I got the movement trails working in the Stalingrad. It took the longest time to figure out what module change "fixed" it. It turns out there is a |
|
@rlament Great ! Does it mean we can go with your solution ? |
|
@Niedowidek If you have confirmed the latest change fixes the behaviour in your module then please mark your approval for this PR. Thanks for the review and leading the way in highlighting the relevant parts of the code. If you had not initiated a PR for this feature request, it would not have happened. Next time pick an easier "good first issue". 😄 |
|
@rlament Works great for areas but doesn't register every position on Volga River (and you said "every position on the trail in the Volga should be visible"). So almost perfect but this Volga river is so hard to cross... ;-) 
|
|
Russian side which doesn't have the option selected registers all the positions on the trail (it is correct). 
|
|
FootprintTest works OK |
Niedowidek
left a comment
Niedowidek
left a comment
There was a problem hiding this comment.
"every position on the trail in the Volga should be visible" still needs to be implemented.
Check if location is offboard instead of checking for null board
|
Volga river movement looks fine. Off-board movement too. I can see that it registers every position on the trail off-board even though they are not visible. 
With the option turned off it works fine too: 
|
|
@rlament I think it works OK now. |
|
Thank you for your help and developing complete solution. |
4 participants
This is an alternate implementation to pull request #14231 which addresses and closes #12051
Add an option to keep the last movement trail position only when location name or mat change.
All off-board movement is always appended to the movement trail.
Check for mat to mat movement within a location.
Add unit tests for movement trails.
Update movement trails documentation adding the new option.
Closes #12051