pret / pokered

Disassembly of Pokémon Red/Blue
4.01k stars 986 forks source link

Label cleanup #315

Open Rangi42 opened 3 years ago

Rangi42 commented 3 years ago
kqesar commented 3 years ago

Hi,

I have several suggestions for this issues, tell me if these are good ideas or not:

What do you think about this @Rangi42 @dannye ?

dannye commented 3 years ago

@KqesaR

In pokecrystal and pokered, we have function setevent and checkevent about event but in pokycrystal the case is not the same that pokered, can be a good things to uniform case between projects ( pokered will have same case convention that pokecrystal)

I agree in general that it's good to try to keep terminology/structure the same between pokered and pokecrystal where possible. But given that the overworld scripts are handled so differently between the two games, I don't think there's much compelling reason to rename the event macros to match. It's not as if scripts could be trivially ported from one game to the other, regardless of whether the event macros are named the same or not.

Can be interesting to name the trainers of gym like pokecrystal ( example, the trainer of PewterGym PewterGymTrainerHeader0 become TrainerCamperJerry)

I don't see why we would introduce our own names for trainers whom have no canonical name.

In map/objects of gym ( for example map/objects/PewterGym.asm) in the def_warps part we can use constant and rename warp to warp_event like pokecrystal: warp 4, 13, 2, LAST_MAP become warp_event 4, 13, PEWTER_CITY, LAST_MAP

I think you misunderstand the pokered warp macro. The third parameter is a warp ID, not a map ID. So warp 4, 13, 2, LAST_MAP would not become warp_event 4, 13, PEWTER_CITY, LAST_MAP. I also don't think there's much value in renaming pokered's warp to warp_event.

Rangi42 commented 3 years ago

I wouldn't mind names like CeladonGymLass1TrainerHeader or FuchsiaGymTamerTrainerHeader. A few are already like that, e.g. ZapdosTrainerHeader (could have been PowerPlantZapdosTrainerHeader too but that's not necessary).

kqesar commented 3 years ago

Thank you for your feedback @Rangi42 @dannye ! It's ok for me :)

kqesar commented 2 years ago

Hello, i have question about case conventions:

I have seen in some scripts that some functions don't have same cases:

For example some function are "PascalCased" like .BagFull or "cameCased" like .beforeBeat.

I have also some functions "snake_cased" like .bag_full in https://github.com/pret/pokered/blob/master/scripts/MrPsychicsHouse.asm#L20

What the good approach ? PascalCase like pokecrystal or camelCase ( if is this approach, can we keep the same conventions in pokecrystal ?)

NB: I can contribute to refactor this if necessary, it's not too hard for me

Thank you in advance