Formatting event constants

[Rangi42]

In the pokecrystal issues I proposed a const_skip macro to help define long constant sequences that have large unused gaps. The events in pokered would be a good use case: of 2,560 constants, only 504 are used. They also look organized by map.

For example:

	const_def
; Pallet Town events
	const EVENT_FOLLOWED_OAK_INTO_LAB
	const EVENT_001
	const EVENT_002
	const EVENT_HALL_OF_FAME_DEX_RATING
	const EVENT_004
	const EVENT_005
	const EVENT_PALLET_AFTER_GETTING_POKEBALLS
	const EVENT_007
	const_skip $018 ; skips 16 events from $008 to $017
	const EVENT_GOT_TOWN_MAP
	const EVENT_ENTERED_BLUES_HOUSE
	const EVENT_DAISY_WALKING
	const EVENT_01B
	const EVENT_01C
	const EVENT_01D
	const EVENT_01E
	const EVENT_01F
	const EVENT_FOLLOWED_OAK_INTO_LAB_2
	const EVENT_OAK_ASKED_TO_CHOOSE_MON
	const EVENT_GOT_STARTER
	const EVENT_BATTLED_RIVAL_IN_OAKS_LAB
	const EVENT_GOT_POKEBALLS_FROM_OAK
	const EVENT_GOT_POKEDEX
	const EVENT_PALLET_AFTER_GETTING_POKEBALLS_2
	const EVENT_OAK_APPEARED_IN_PALLET
; Viridian City events
	const_skip $028 ; skips 0 events  [this isn't necessary, but would be possible if we want to indicate the start of the Viridian City events]
	const EVENT_VIRIDIAN_GYM_OPEN
	const EVENT_GOT_TM42
	const_skip $038 ; skips 14 events from $02A to $037
	const EVENT_OAK_GOT_PARCEL
	const EVENT_GOT_OAKS_PARCEL
	const_skip $050 ; skips 22 events from $03A to $04F
	const EVENT_GOT_TM27
	const EVENT_BEAT_VIRIDIAN_GYM_GIOVANNI
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_0
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_1
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_2
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_3
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_4
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_5
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_6
	const EVENT_BEAT_VIRIDIAN_GYM_TRAINER_7
; Pewter City events
	const_skip $068 ; skips 14 events from $05A to $067
	const EVENT_BOUGHT_MUSEUM_TICKET
	const EVENT_GOT_OLD_AMBER
	const_skip $072 ; skips 8 events from $06A to $071
	const EVENT_BEAT_PEWTER_GYM_TRAINER_0
	const EVENT_073
	const EVENT_074
	const EVENT_075
	const EVENT_GOT_TM34
	const EVENT_BEAT_BROCK
; Cerulean City events
	const_skip $098 ; skips 32 events from $078 to $097
	const EVENT_BEAT_CERULEAN_RIVAL
	[etc]

(Maybe it would look better with the ; skips N events comments on their own lines above the const_skips.)

The ; X City events comments would also be a convenient place to mention the current address (whether as a raw hex value or as an offset from wEventFlags), without needing a comment on every single EVENT_* line.

[Rangi42]

There's some subjectivity in which ranges of unused constants to skip. For example, the Pallet Town events could have skipped the 5 ones from EVENT_01B to EVENT_01F. And the Pewter City events could have been more byte-oriented:

; Pewter City events
	const_skip $068 ; skips 14 events from $05A to $067
	const EVENT_BOUGHT_MUSEUM_TICKET
	const EVENT_GOT_OLD_AMBER
	const_skip $070 ; skips 6 events from $06A to $06F
	const EVENT_070
	const EVENT_071
	const EVENT_BEAT_PEWTER_GYM_TRAINER_0
	const EVENT_073
	const EVENT_074
	const EVENT_075
	const EVENT_GOT_TM34
	const EVENT_BEAT_BROCK

[dannye]

I think this would improve the readability a lot, and having them grouped by map is nice.

As far as the subjective things, I think it's okay to not skip a few unused events which are sandwiched in the middle of a map as long as there are not too many (perhaps less than 8).
I don't think it's too helpful to keep the skipped events byte-aligned.
I'm on the fence about skipping 0 events at the start of a map - leaning towards saying it's not necessary handy.

Two questions/concerns:

Will the const_skip macro error/warn when the new ID skips backwards?

The way things are now, it's very easy to add new events and be fully confident that you have not accidentally shifted old events up or down - because you just edit existing lines rather than add new lines.
With this proposal, adding events will be slightly less trivial. Could there be a way to automatically and easily verify that the values of old event constants are unaffected? (EDIT: I guess the skipping 0 events could help with this actually.)

[Rangi42]

Yes, it would prevent skipping backwards; details are in pret/pokecrystal#739 (comment).

const_skip is like const_def in that it lists constants starting from a fixed address, so you can safely append more events to the end of any series (i.e. before the next const_skip). Just like before, if you insert a new event between two existing ones, that would shift at least some events' values. It won't be possible to shift by appending too many events before a const_skip, because that would require skipping backwards and will fail.

Since there are so many unused events available, it's probably better to encourage inserting new events into the unused gaps, than to append them after all the rest of the events (unlike with adding a new sprite, item, move, etc). So there could be a fixed NUM_EVENTS EQU $A00 at the bottom, instead of NUM_EVENTS EQU const_value, with an assert NUM_EVENTS == const_value, "Events have shifted!" to make sure they don't shift from adding or removing constants.