RFC: Warps using tiles instead of pixels. (includes patch)

[wushin]

Instead of requiring warps to use pixels, I suggest using my patch to enable use of either or. I'd imagine as time wears on using tiles will become the preferred method.

The map properties dest_x and dest_y would remain the same and be assumed to be pixel based warps.
The new map properties dtile_x and dtile_y would denote tile based warps and always assumed so.

I know this seems minor but it causes a lot of inaccuracy and bugfixes.

[Bjørn]

The values are in pixels because manaserv is dealing with pixels, since this is the warp format understood by manaserv and only later was the converter written to make warps defined in Tiled work with tmwAthena as well.

Of course, I have no problem with adding an additional pair of properties for tile coordinates. I'd call it dest_tile_x rather than dtile_x though, simply cause I think that's more understandable.

Note that in manaserv we have done away with specifying destination coordinates completely, by allowing another warp object to be specified as the destination. This makes it easier to set up a warp and to understand where a warp is going, as well as avoiding issues with later map changes. This may be something you could consider supporting in the converter as well. More information at http://doc.manasource.org/mappingref:warp.

[wushin]

Changed the vars to bjorns sug. I like the way the warps in manaserv work and it would be helpful. Short term, I think this patch reaps more low-hanging fruit.

[o11c]

Technical review:

There is no need for a separate fdest_x field. There is no need to use setattr, since the field name is constant.

Instead do something like:

Code: Select all

if not hasattr(obj, 'dest_tile_x'):
    obj.dest_tile_x = obj.dest_x / 32

I would also like it to be strict about having exactly one set of coordinates.

[wushin]

I hope this is correct. I think I did the assert correctly.

[o11c]

That part looks good, but:

• You forgot to remove the 'fdest_x' lines from the slots section
• In Python, don't put () in an if statement.

Also can you put this on a branch of your github?

[wushin]

fixed all the above.
Here is the Git https://github.com/wushin/tmwa-server-d ... arp-change

[o11c]

One of the lines has trailing whitespace.

The body of the if statement should be indented by 4 more spaces, not 2.

I have now actually run it. The print() statement should not be added.

The branch has one commit locally that is not in the main repo - it looks like something that was rebased into the main repo. I can deal with this, but it might be indicative of a greater problem in your repo or workflow.

[wushin]

Yeah, I still kinda sloppy. I'm an old guy who used svn for years and years cause git didn't exist. I'm getting better, ask tux or jena.
I think I see the offending commit Nivalis warp fixes.
https://github.com/wushin/tmwa-server-d ... 3ce7faf496

So does this mean your going to take it on home? " I can deal with this" - o11c