Use named constants for warp IDs?

[Rangi42]

For example, here's how some would look:

; Route32.asm:
	def_warp_events ROUTE_32
	warp_event 11, 73, ROUTE_32_POKECENTER_1F, LEFT
	warp_event  4,  2, ROUTE_32_RUINS_OF_ALPH_GATE, UPPER
	warp_event  4,  3, ROUTE_32_RUINS_OF_ALPH_GATE, LOWER
	warp_event  6, 79, UNION_CAVE_1F

; Route32RuinsOfAlphGate.asm:
	def_warp_events ROUTE_32_RUINS_OF_ALPH_GATE
	warp_event  0,  4, RUINS_OF_ALPH_OUTSIDE, UPPER
	warp_event  0,  5, RUINS_OF_ALPH_OUTSIDE, LOWER
	warp_event  9,  4, ROUTE_32, UPPER
	warp_event  9,  5, ROUTE_32, LOWER

; UnionCave1F.asm:
	def_warp_events UNION_CAVE_1F
	warp_event  5, 19, UNION_CAVE_B1F, A
	warp_event  3, 33, UNION_CAVE_B1F, B
	warp_event 17, 31, ROUTE_33
	warp_event 17,  3, ROUTE_32

And here are the constants those macros would be defining and using:

; Route32.asm:
	def_warp_events ROUTE_32
	; DEF WARP__ROUTE_32__ROUTE_32_POKECENTER_1F__LEFT EQU 1
	warp_event 11, 73, ROUTE_32_POKECENTER_1F, LEFT ; db WARP__ROUTE_32_POKECENTER_1F__ROUTE_32__LEFT
	; DEF WARP__ROUTE_32__ROUTE_32_RUINS_OF_ALPH_GATE__UPPER EQU 2
	warp_event  4,  2, ROUTE_32_RUINS_OF_ALPH_GATE, UPPER ; db WARP__ROUTE_32_RUINS_OF_ALPH_GATE__ROUTE_32__UPPER
	; DEF WARP__ROUTE_32__ROUTE_32_RUINS_OF_ALPH_GATE__LOWER EQU 3
	warp_event  4,  3, ROUTE_32_RUINS_OF_ALPH_GATE, LOWER ; db WARP__ROUTE_32_RUINS_OF_ALPH_GATE__ROUTE_32__LOWER
	; DEF WARP__ROUTE_32__UNION_CAVE_1F EQU 4
	warp_event  6, 79, UNION_CAVE_1F ; db WARP__UNION_CAVE_1F__ROUTE_32

; Route32RuinsOfAlphGate.asm:
	def_warp_events ROUTE_32_RUINS_OF_ALPH_GATE
	; DEF WARP__ROUTE_32_RUINS_OF_ALPH_GATE__RUINS_OF_ALPH_OUTSIDE__UPPER EQU 1
	warp_event  0,  4, RUINS_OF_ALPH_OUTSIDE, UPPER ; db WARP__RUINS_OF_ALPH_OUTSIDE__ROUTE_32_RUINS_OF_ALPH_GATE__UPPER
	; DEF WARP__ROUTE_32_RUINS_OF_ALPH_GATE__RUINS_OF_ALPH_OUTSIDE__LOWER EQU 2
	warp_event  0,  5, RUINS_OF_ALPH_OUTSIDE, LOWER ; db WARP__RUINS_OF_ALPH_OUTSIDE__ROUTE_32_RUINS_OF_ALPH_GATE__LOWER
	; DEF WARP__ROUTE_32_RUINS_OF_ALPH_GATE__ROUTE_32__UPPER EQU 3
	warp_event  9,  4, ROUTE_32, UPPER ; db WARP__ROUTE_32__ROUTE_32_RUINS_OF_ALPH_GATE__UPPER
	; DEF WARP__ROUTE_32_RUINS_OF_ALPH_GATE__ROUTE_32__LOWER EQU 4
	warp_event  9,  5, ROUTE_32, LOWER ; db WARP__ROUTE_32__ROUTE_32_RUINS_OF_ALPH_GATE__LOWER

; UnionCave1F.asm:
	def_warp_events UNION_CAVE_1F
	; DEF WARP__UNION_CAVE_1F__UNION_CAVE_B1F__A EQU 1
	warp_event  5, 19, UNION_CAVE_B1F, A ; db WARP__UNION_CAVE_B1F__UNION_CAVE_1F__A
	; DEF WARP__UNION_CAVE_1F__UNION_CAVE_B1F__B EQU 2
	warp_event  3, 33, UNION_CAVE_B1F, B ; db WARP__UNION_CAVE_B1F__UNION_CAVE_1F__A
	; DEF WARP__UNION_CAVE_1F__ROUTE_33 EQU 3
	warp_event 17, 31, ROUTE_33 ; db WARP__ROUTE_33__UNION_CAVE_1F
	; DEF WARP__UNION_CAVE_1F__ROUTE_32 EQU 4
	warp_event 17,  3, ROUTE_32 ; db WARP__ROUTE_32__UNION_CAVE_1F

(The macros would explicitly check for a "-1" argument to warp to "the last map", like elevators do.)

Pros: People wouldn't get confused about what the number means. Rearranging warps would Just Work automatically.

Cons: More obscure magic going on behind the scenes. Warps to the same map, like in Saffron Gym, would need some kind of exception. (Maybe a warp_event_self x, y, from id, to id?)

[Rangi42]

Never mind, this also wouldn't neatly handle the case where multiple warps share a destination (e.g. two carpet sides inside a building going to one door outside).

[mid-kid]

I do think it'd be good to have named constants for warps regardless.

[mid-kid]

I think last time, I shot something like this down out of concerns for greppability, I now think that's a non-issue when it comes to this, as you can simply look for the map (script) name.

[Rangi42]

In that case, let's try and work out how non-injective and self-warps could work.

Currently:

; SafronCity.asm:
	def_warp_events
	warp_event 26,  3, FIGHTING_DOJO, 1
	warp_event 34,  3, SAFFRON_GYM, 1
	...

; SaffronGym.asm:
	def_warp_events
	warp_event  8, 17, SAFFRON_CITY, 2
	warp_event  9, 17, SAFFRON_CITY, 2
	warp_event 11, 15, SAFFRON_GYM, 18
	warp_event 19, 15, SAFFRON_GYM, 19
	...
	warp_event 19, 17, SAFFRON_GYM, 3
	warp_event 19,  9, SAFFRON_GYM, 4
	...

[mid-kid]

I think something like warp_def x, y, source_name, target_map, target_name could work

[Rangi42]

How about this:

; SafronCity.asm:
	def_warp_events SAFFRON_CITY
	warp_event 26,  3, FIGHTING_DOJO, LEFT
	warp_event 34,  3, SAFFRON_GYM, LEFT
	...

; SaffronGym.asm:
	def_warp_events SAFFRON_GYM
	warp_event  8, 17, SAFFRON_CITY, LEFT
	warp_event  9, 17, SAFFRON_CITY, RIGHT, LEFT
	warp_event 11, 15, SAFFRON_GYM, PANEL_18, PANEL_3
	warp_event 19, 15, SAFFRON_GYM, PANEL_19, PANEL_4
	...
	warp_event 19, 17, SAFFRON_GYM, PANEL_3, PANEL_18
	warp_event 19,  9, SAFFRON_GYM, PANEL_4, PANEL_19
	...

Basically, the def_warp_events macro now takes an arg to save as SOURCE. Then we'd have:

• 3-arg warp_event X, Y, TARGET: Defines WARP__SOURCE__TARGET and uses WARP__TARGET__SOURCE (must be a unique warp between those two maps)
• 4-arg warp_event X, Y, TARGET, ID: Defines WARP__SOURCE__TARGET__ID and uses WARP__TARGET__SOURCE__ID (must be a mutual warp between those two maps)
• 5-arg warp_event X, Y, TARGET, SRC_ID, TGT_ID: Defines WARP__SOURCE__TARGET__SRC_ID and uses WARP__TARGET__SOURCE__TGT_ID

In all those cases, an ID of -1 is used literally and does not cause any definitions. So elevators would work like this:

; GoldenrodDeptStore1F.asm:
	def_warp_events GOLDENROD_DEPT_STORE_1F
	...
	warp_event  2,  0, GOLDENROD_DEPT_STORE_ELEVATOR, LEFT

; GoldenrodDeptStore2F.asm:
	def_warp_events GOLDENROD_DEPT_STORE_2F
	...
	warp_event  2,  0, GOLDENROD_DEPT_STORE_ELEVATOR, LEFT

; GoldenrodDeptStoreElevator.asm:
	def_warp_events GOLDENROD_DEPT_STORE_ELEVATOR
	warp_event  1,  3, GOLDENROD_DEPT_STORE_1F, LEFT, -1
	warp_event  2,  3, GOLDENROD_DEPT_STORE_1F, RIGHT, -1

[Rangi42]

@dannye Thoughts on this system as applied to pokered?

[Rangi42]

Bikeshedding: Maybe the order should be X, Y, TARGET, TGT_ID, SRC_ID instead. Or even X, Y, SRC_ID, TARGET, TGT_ID?

[Rangi42]

@vulcandth Curious what you think too, you did all those other pokered map event constants.

[mid-kid]

I prefer explicit over implicit in this case. I don't think it's gonna be clear to the casual eye what the 3-arg and 4-arg variants do. (And map scripts are prime material for casuals to wander into)

Instead, I'd consider whether we should follow the example of the object event defines at the top of the file.

[Rangi42]

Okay, interesting. Because previously I've proposed the opposite, to define object event constants along with their events, not at the opposite end of the file.

Could you outline how you see the Route32+Route32RuinsOfAlphGate+UnionCave1F example working that way?

[Rangi42]

(Despite all the above tinkering, I still kind of want to just stick with the current raw-number system for its simplicity, even if it takes more manual counting to work with.)

[Rangi42]

I think something like warp_def x, y, source_name, target_map, target_name could work

@mid-kid I think you're right. Also in chat recently I was liking the idea of lowercase map identifiers, otherwise it's too much SHOUTING on one line.

So the above example would be:

; Route32.asm:
	def_warp_events ROUTE_32
	warp_event 11, 73, pokecenter, ROUTE_32_POKECENTER_1F, left
	warp_event  4,  2, alph_top, ROUTE_32_RUINS_OF_ALPH_GATE, top_right
	warp_event  4,  3, alph_bottom, ROUTE_32_RUINS_OF_ALPH_GATE, bottom_right
	warp_event  6, 79, union_cave, UNION_CAVE_1F, route_32

; Route32RuinsOfAlphGate.asm:
	def_warp_events ROUTE_32_RUINS_OF_ALPH_GATE
	warp_event  0,  4, top_left, RUINS_OF_ALPH_OUTSIDE, route_32_top
	warp_event  0,  5, bottom_left, RUINS_OF_ALPH_OUTSIDE, route_32_bottom
	warp_event  9,  4, top_right, ROUTE_32, alph_top
	warp_event  9,  5, bottom_right, ROUTE_32, alph_bottom

; UnionCave1F.asm:
	def_warp_events UNION_CAVE_1F
	warp_event  5, 19, a, UNION_CAVE_B1F, a
	warp_event  3, 33, b, UNION_CAVE_B1F, b
	warp_event 17, 31, route_33, ROUTE_33, union_cave
	warp_event 17,  3, route_32, ROUTE_32, union_cave

(This format also has an advantage for Polished Map: it could easily distinguish between the old 4-arg and new 5-arg formats, and still be able to Shift+click a warp to open its target map.)

[DamienDoury]

Here is my take:

I think some kind of label system would be a good replacement for warp IDs.
Labels could be moved around easily, and are easy to identify, which would help with code readability and code searches.

Here is an example:
In AzaleaTown.asm

def_warp_events
AzaleaTown_Warps:
.SlowpokeWellEntrance:
warp_event 31,  7, SlowpokeWellB1F_Warps.Exit
.PokecenterEntrance
warp_event 15,  9, AzaleaPokecenter1F.LeftExit
...

And in SlowpokeWellB1F:

def_warp_events
SlowpokeWellB1F_Warps:
.Exit:
warp_event 17, 15, AzaleaTown_Warps.SlowpokeWellEntrance
...

Feel free to edit this idea to your liking.
The label should be converted to an index at compile time, so it stays 100% compatible with vanilla.

[Rangi42]

Thank you for writing up your proposal!

Basically instead of this:

	def_warp_events <CUR_MAP>
	warp_event <X>, <Y>, <Warp1Name>, <DEST_MAP_1>, <DestWarp1Name>
	warp_event <X>, <Y>, <Warp2Name>, <DEST_MAP_2>, <DestWarp2Name>

You're proposing this:

<CurMap>_Warps:
	def_warp_events
	.<Warp1Name>:
	warp_event <X>, <Y>, <DestMap1>_Warps.<DestWarp1Name>
	.<Warp2Name>:
	warp_event <X>, <Y>, <DestMap2>_Warps.<DestWarp2Name>

This doesn't seem all that different to me from the way object_event constants are defined separately from the events themselves, except this is using labels instead of constants. For instance, we could imagine:

	warp_const_def <CUR_MAP>
	def_warp_events
	warp_const <Warp1Name>
	warp_event <X>, <Y>, <DEST_MAP_1>, <DestWarp1Name>
	warp_const <Warp2Name>
	warp_event <X>, <Y>, <DEST_MAP_2>, <DestWarp2Name>

Or even separate them into two lists, like how object_const_def are at the top and def_object_events at the bottom:

	warp_const_def <CUR_MAP>
	warp_const <Warp1Name>
	warp_const <Warp2Name>

	def_warp_events
	warp_event <X>, <Y>, <DEST_MAP_1>, <DestWarp1Name>
	warp_event <X>, <Y>, <DEST_MAP_2>, <DestWarp2Name>

Separating the constants from the other event data kind of makes sense for object_events, because they have so much data and the lines are too long already; but warp_events wouldn't be all that long, even if done in one line.

(Also, given that the warp_event macro expands to use a map_id, we have to get the all-caps CUR_MAP and DEST_MAP in there somehow; so if the label idea is just to reduce SHOUTING, well, it would still have to put those in caps.)

[DamienDoury]

I think we shouldn't separate the labels and data into 2 lists, as it makes editing warps more prone to errors.
By separating them, we are kinda going back to an index system.

For example, if someone is not careful in a big list of warps, they could remove the warp label at index 6 and warp data at index 7.

In my opinion, it's important that the label stays next to the data.

That's why I think

	warp_const_def <CUR_MAP>
	def_warp_events
	warp_const <Warp1Name>
	warp_event <X>, <Y>, <DEST_MAP_1>, <DestWarp1Name>
	warp_const <Warp2Name>
	warp_event <X>, <Y>, <DEST_MAP_2>, <DestWarp2Name>

Is a better solution.
Also, words in CAPS is not an issue to me.