[b87] small bug: getMoveCount()

[wolph42]

I just encountered a small bug in getMoveCount(): it turns out that if you run this macro for a 'fresh' token (one you just dragged from the lib onto the map) then you get an 'invalid expression' error. I think this should be 0.

It turns out that getMoveCount() only works AFTER you have moved the token at least once. I can understand why this is happening, but I don't think this should be happening.

[jfrazierjr]

I just encountered a small bug in getMoveCount(): it turns out that if you run this macro for a 'fresh' token (one you just dragged from the lib onto the map) then you get an 'invalid expression' error. I think this should be 0.

It turns out that getMoveCount() only works AFTER you have moved the token at least once. I can understand why this is happening, but I don't think this should be happening.

Interesting. in the same vein, I wonder what getLastPath() does also(ie, would you please check for me...)

[aliasmask]

As a reminder, Wiki: getLastPath() is broken when moving multiple tokens where when the "key" token is not snapped to grid, the snapped tokens are reported to be in the wrong place:

Name: Carnage Demon
Last Path: [{"x":100,"y":500},{"x":200,"y":500}]
Start: {"x":100,"y":500}
Final Position: {"x":200,"y":500}
Name: Demonic Ape
Last Path: [{"x":100,"y":501},{"x":200,"y":501}]
Start: {"x":100,"y":600}
Final Position: {"x":200,"y":600}

See how Last Path is different when Carnage Demon is not snap to grid and it is selected to be moved, while Demonic Ape is snapped to grid. If I move the Demonic Ape instead (which is snapped to grid), then lastPath is done correctly. It's a symptom of the "key" token and applying the change in position of the other tokens and using the wrong scale. The 501, which should be 600 is the key token 500 + 1 (grid square), rather than is actual position of 500 + 100 (map points) in the y direction.

The error still occurs if both tokens are unsnapped. The error goes away if the map is gridless. The error goes away if the key token moved is snapped to grid.

Here is the test code and example campaign to test for yourself. Because the current position of a token when onTokenMove is the last position in lastPath, I need to save the x,y values to the token so I could determine the starting position before onTokenMove was called. I also would have put the snap state of a token in the output, but there is no macro to check for that.

/// TEST CODE ///

Code: Select all

@@ @getXY
[H: x = getTokenX()]
[H: y = getTokenY()]
[H: setProperty("x",x)]
[H: setProperty("y",y)]
[H: data = json.set(macro.args,"Final Position",json.set("{}","x",x,"y",y))]
[H: output = "<table style='padding:0px'>"]
[H, foreach(key,data): output = json.append(output,strformat("<tr style='padding:0px'><td style='padding:0px'><b>%{key}: </b></td><td style='padding:0px'>%s</td></tr>",json.get(data,key)))]
[R: broadcast(json.toList(output,"")+"</table>")]

!!
@@ @onTokenMove
[H: lastPath = getLastPath()]
[H: startPosition = json.set("{}","x",getProperty("x"),"y",getProperty("y"))]
[H: link = macroLinkText("getXY@lib:test","none",json.set("{}","Name",token.name,"Last Path",lastPath,"Start",startPosition),currentToken())]
[H: execLink(link,1)]

!! 

[aliasmask]

I just encountered a small bug in getMoveCount(): it turns out that if you run this macro for a 'fresh' token (one you just dragged from the lib onto the map) then you get an 'invalid expression' error. I think this should be 0.

It turns out that getMoveCount() only works AFTER you have moved the token at least once. I can understand why this is happening, but I don't think this should be happening.

Interesting. in the same vein, I wonder what getLastPath() does also(ie, would you please check for me...)

lastPath returns [] when dropped on to map, so that is okay. lastPath is also cleared if tokens.denyMove is set to 1 in onTokenMove or onMultiTokenMove, which I think is okay.

[wolph42]

well, there's your answer. So its only getMoveCount() and apparently getLastPath() as well.

[Lee]

@A.M. As I mentioned on the Testing section, the "key" token always reports correct coordinates regardless of snapped status and the "follower" tokens always end up wrong. It might be that I'm mistaken, but when you mean "key", you mean the token that is being dragged, correct?

Edit: Also, I need a little enlightenment. Based on the wiki guide to onTokenMove, it mentions that when more than 1 token is moved, both onTokenMove and onMultipleTokensMove trigger. Why? I get that it gives to hooks for use, but doesn't it also create redundancy somewhere and, potentially, conflicts as well?

@wolph I'll wait until after Jamz submits his patches as I think he touched the Token class as well. Once it gets reviewed and, hopefully, committed, I'll work on finalizing inclusion of the default path to a token's initial properties.

[Lee]

Status update on A.M.s bug: I got around this morning to look at it and it is now fixed. There is, sadly, a loss of precision on situations where an unsnapped follower token is following a snapped key token. The loss happens when its actual coordinates get converted to cell coordinates in the computation of the offset from a key token's footprint, e.g. 437, 287 -> 4, 2. So, when a function like getLastPath retrieves path info, the cell points get computed back to zone points becoming: 4, 2 -> 400, 200.

Converting the system to double would solve this problem, but "refactoring" the MT code for this change, is painful in terms of casting/switching data types. The underlying math will likely work as it is and thus, will remain untouched. The question is, would the effort be worth it?

Also, I'm trying to come up with a way to create actual footprints for follower tokens following an unsnapped key token. Right now, the path is computed based on endpoints and waypoints. Let me know if this is good enough and I'll upload the patch. In the meantime, I'm pretty sure the logic for creating footprints when a snapped token is dragged can be leveraged into the path creation for a follower token; but I won't be working on that right now since I'm mopping up something else for the moment.

[Lee]

patch uploaded to Testing.

[Azhrei]

Would it be possible to keep the actual coordinates and convert to CellPoint on the fly as needed? Then in the last step, when the tokens are dropped on the map, the original offset would still be there?

I don't remember the details of that code so I can't speak intelligently on it right now (I'm supposed to be working ).

[Lee]

While the answer is yes, and conversion is already on the fly in the original, the mere act of converting destroys the precision. For example 327, 556 becomes either 3, 5 or 3,6. When these values pass through the portion where the value for getLastPath is returned, AbstractPoints in the form of CellPoints get multiplied up by 100, so without doing what I did, we'll always see values of 100 (e.g. 300, 500).

Early on, I switched to double for the heck of it. The resultant red flood on the project tree made me undo that really quick; it would've worked quite well in preserving precision without changing the original code, however. My next attempt was to use a combination the slope of the line to find points along the line segment made by start and end, and static incrementation when slope=0. This was met with mixed success because, again, the loss of precision. Another complicating factor is that the whole group is reliant on the path made by the leading token; many of my attempts to break away from this dependency returned me to the conclusion that handling each mixed context was needed to have a modicum of precision. As can be seen, I also had to change up the order and location of some function calls to get to the goal. It wasn't my favorite type of coding problem, to be honest.

When I first posted news of a fix that came before my attempt to reintroduce precision, all path returns were actually cell paths * 100 and this was accomplished with a smaller impact to the original code structure. I guess I could backtrack to that but perhaps the community would in the end be the one to let us know which one they prefer.

[aliasmask]

Well, the only place that I know of that the path is a problem is with the onTokenMove event. The path is always in x,y coords (rather than grid units). Is there anyway to just force it to use x,y coords when determining paths just for that (ie force unsnapped math)?

[Lee]

The qualifier for that would be, would it be easy? The answer is no.

Prior to the changes I've made, there was no such thing as "unsnapped math"; everything was, and still is, reliant on Cell (point) based computations, which is neater for most applications as far as I can tell. The only nod for an unsnapped path was for it to have the start and end point of the token's "dragging".

I still think creating a path based on slope of a line will generate the best results, what was left to make it succeed is finding the magnitude of a step (i.e. standard cell spacing is at 100) that will coincide with an intersection; a headache giver at that time because we're mixing integers with results in double. Basically, it needed a round result using a point within the coverage of a token's area in combination with either the start or end point. I believe I was close to solution, but I didn't want to get mired over just one problem when there were other fish to fry.

So, I honestly dropped it in favor of doing something else (e.g. UPnP). It was only the other day that I decided to implement plan C to get footprints for unsnapped solo movement. I'd like to ask, how's it holding up so far?

[Azhrei]

Hm. I guess my question could be better phrased... There is an (x,y) ZonePoint in the token already, right? This would be recorded using Zone coordinates, not Cell coordinates.

If the offset from the base token to the others in the group were calculated from that, then when the group was moved and the base token dropped, the delta value could be reapplied to the other (unsnapped) tokens. Since those other tokens are unsnapped, the arithmetic would use ZonePoints.

I know when Trevor first implemented the unsnapped concept it was grafting on a lot of code to what was already there. It was quite a kludge. You can see that in how IF statements are used prior to invoking the A* walker, for example. It would be better (for some subjective definition of "better"!) if there were an A* walker that simply using start and end points because then the code that handles movement could delegate to the Walker and move the unsnapped token rules into a separate class. I looked into doing that, IIRC, but it turned out to require a lot of plumbing changes and was too intrusive.

[Lee]

Hm. I guess my question could be better phrased... There is an (x,y) ZonePoint in the token already, right? This would be recorded using Zone coordinates, not Cell coordinates.

If the offset from the base token to the others in the group were calculated from that, then when the group was moved and the base token dropped, the delta value could be reapplied to the other (unsnapped) tokens. Since those other tokens are unsnapped, the arithmetic would use ZonePoints.

Ah, yes, ideally. Unfortunately, the original code only handled CellPoint based movement, from the point the Token is "lifted", all coords already go through CellPoint conversion; there is nothing in between start and end that gives a nod to ZonePoint based movement, apart from the conversion back to ZP for display, of course. Even the deltas were in CP. I believe this was to make it easier to support the current system of multiple token movement where follower tokens depend on the key token's path to know their own.

Everything using ZP in my patch, I had to put in myself. There is nothing in the code for unsnapped movement beyond putting the start and end points in the path, and (especially) there is no walker for it. In a conversation about it, JFJ admitted that unsnapped movement computation wasn't there because it was a "PITA"

The second impediment is that to handle precise unsnapped movement, coordinate values must hold precision values. To do as you mentioned above, would be impossible (though I'm no math genius) with integer values when computing a point in the path/line from the center of the token. I did think before that if, instead, a point within the token (not center) is acceptable as being "on the path", then squaring the circle could create a sort of pseudo square-grid movement for a walker to be written from; though the squares (in my mind's eye) would seem to need to shrink in proportion as the slope of the line approaches 0.

I settled for what I did in the patch

I know when Trevor first implemented the unsnapped concept it was grafting on a lot of code to what was already there. It was quite a kludge. You can see that in how IF statements are used prior to invoking the A* walker, for example. It would be better (for some subjective definition of "better"!) if there were an A* walker that simply using start and end points because then the code that handles movement could delegate to the Walker and move the unsnapped token rules into a separate class. I looked into doing that, IIRC, but it turned out to require a lot of plumbing changes and was too intrusive.

Indeed, that would be a better way. I also think that, in the future, MT should break away from the current model of using only the key token's path to get theirs, and instead, make it to each their own, regardless of the scenario. Sure, at least theoretically, the way things are done now would be computationally faster, though when I see how everything is done iteratively, it wouldn't be by much IMO.

While we're theorizing, why not a heap-killing algorithm that processes individual path computation of a moving mass (all or by batches) of tokens in parallel? An old-system choker for sure Maybe for 1.4, we can consider using precision values for the coordinate system. If we had that, we can keep the original path code which works great for snapped movement, we just have to use the significant value left of the decimal (after grid conversion) for cell offset; while finally injecting point intersection with the slope of the path for unsnapped tokens. For the latter we can then include in functions (e.g. getLastPath) a parameter to control the magnitude between points to avoid returning infinity.

[Azhrei]

Unfortunately, the original code only handled CellPoint based movement, from the point the Token is "lifted", all coords already go through CellPoint conversion; there is nothing in between start and end that gives a nod to ZonePoint based movement, apart from the conversion back to ZP for display, of course.

Are you sure? How are unsnapped tokens handled when the primary token is unsnapped? Or when the map doesn't support a grid (i.e. "gridless movement")?

Indeed, that would be a better way. I also think that, in the future, MT should break away from the current model of using only the key token's path to get theirs, and instead, make it to each their own, regardless of the scenario.

Well, taking that a little further... Each token should not only move independently via their own ZoneWalker but there should be constraints on how that movement should be allowed. For example, if I grab tokens that are Medium and Huge then drag them through an area where the Huge won't fit, what happens? What about games where two units may not occupy the same square at the same time -- how do we tell the ZoneWalker that? Or maybe units can occupy the same square but only if they overlap by 50% or less? What about the yet-to-be-implemented "movement blocking layer"? If the MBL is implemented as some kind of terrain movement modifier (seems likely) then the ZoneWalker might have to route around such areas and that can result in overlapping (as above) or in some tokens not being able to move as far as others. In cases where a slower unit can't move as far, it might make more sense to route it differently (game mechanics will dictate that) so how should MT handle that scenario?

Even if we don't fix all those issues there should be a discussion of what the issues are -- that way we can factor them in as we design and code any future ZoneWalker class...

Moving units as a group is only a convenience, AFAIC. It shouldn't factor into game mechanics at this point in the development of MT because there are too many unresolved (and even undiscussed) issues. I'd be fine with all unit movement acting as though all tokens were unsnapped, but I know that because of the way it works now such a change would make a lot of people unhappy. Of course, those same people are already unhappy that MT does have "token grouping", where tokens not only move as a group but can intelligently turn corners and have a "group centerpoint" used for rotation!!