Use named constants for warp IDs? about pokecrystal HOT 16 OPEN

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

from pokecrystal.

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

from pokecrystal.

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).

from pokecrystal.

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.

from pokecrystal.

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
	...

from pokecrystal.

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

from pokecrystal.

@dannye Thoughts on this system as applied to pokered?

from pokecrystal.

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

from pokecrystal.

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

from pokecrystal.

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.

from pokecrystal.

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?

from pokecrystal.

(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.)

from pokecrystal.

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.)

from pokecrystal.

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.

from pokecrystal.

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.)

from pokecrystal.

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.

from pokecrystal.