Incidencia #44166

Phase out direct references to alltemperate and single_pole outside mapgen

Abrir Fecha: 2022-03-23 23:04 Última actualización: 2022-03-25 19:17

Informador:
Propietario:
Tipo:
Estado:
Cerrado
Componente:
Hito:
Prioridad:
5 - Medium
Gravedad:
5 - Medium
Resolución:
Fixed
Fichero:
2

Details

Required for #44038. Introduce macros for northern- and southernmost latitude in map.h, as well as potentially other helper macros, and use those where applicable.

This ticket is only for common code; since making those changes in mapgen is more involved, that will happen separately in #44167

Ticket History (3/10 Histories)

2022-03-23 23:04 Updated by: alienvalkyrie
  • New Ticket "Phase out direct references to alltemperate and single_pole outside mapgen" created
2022-03-23 23:26 Updated by: alienvalkyrie
  • Resolución Update from Ninguno to Accepted
  • Details Updated
Comentario

Note: The MAP_MAX_ABS_LATITUDE and MAP_MIN_ABS_LATITUDE macros introduced in this patch aren't used yet, but will be in #44167.

2022-03-23 23:35 Updated by: cazfi
Comentario

This is not an opinion to either direction, but something that you may want to consider: Should the macros support maps other than main wld.map, i.e., to take the map as parameter? Though for the map properties these macros are interested about are very unlikely to ever differ between active maps (-> values from wld.map are fine for any other map as well)

2022-03-24 00:09 Updated by: alienvalkyrie
Comentario

Reply To cazfi

Should the macros support maps other than main wld.map, i.e., to take the map as parameter?

I thought about that too, and I'm a bit torn – on the one hand, it likely won't be necessary for a long time (maybe ever) and might make relevant code less readable with additional (wld.map)s left and right; on the other hand, it would be more versatile and future-proof, and it might help clearly separate global constants from things depending on an individual map. I think I'll check how it affects code readability, and if that's fine, I'll do it. After all, explicit is better than implicit.

2022-03-24 00:31 Updated by: alienvalkyrie
Comentario

Reply To alienvalkyrie

Reply To cazfi

Should the macros support maps other than main wld.map, i.e., to take the map as parameter?

I think I'll check how it affects code readability, and if that's fine, I'll do it.

While it doesn't make the ever-constant struggle with line lengths and code layout easier (particularly in some parts of the stuff for #44167), it's not too bad, so there's really no reason not to do it.

Also, this new patch adds one more macro that'll be useful for #44167, so that all changes for that ticket will be contained to mapgen.

2022-03-24 00:45 Updated by: cazfi
Comentario

Reply To alienvalkyrie

While it doesn't make the ever-constant struggle with line lengths and code layout easier (particularly in some parts of the stuff for #44167), it's not too bad, so there's really no reason not to do it.

We'll see how long it takes until another round of refactoring introduces this:

#define WLD_NORTH_LATITUDE MAP_NORTH_LATITUDE(wld.map)

2022-03-24 01:03 Updated by: alienvalkyrie
Comentario

Reply To cazfi

We'll see how long it takes until another round of refactoring introduces this:
#define WLD_NORTH_LATITUDE MAP_NORTH_LATITUDE(wld.map)

I mean, yeah, might be sensible to do that in general – make all the stuff in map.[ch] take a map parameter, and declare WLD_ versions for all of it; it'd certainly be clearer than the current mish-mash situation. But that'd be a large undertaking best left for when we have more use for it, or have a couple days with nothing else to do (or switch to a programming language that makes it easier... fat chance).

2022-03-25 19:17 Updated by: alienvalkyrie
  • Estado Update from Open to Cerrado
  • Resolución Update from Accepted to Fixed

Editar

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Entrar