Refactor tilespec.cpp #450
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lmoureaux
force-pushed
the
feature/split-tilespec-cpp
branch
4 times, most recently
from
6fc35fd to
1fab1f6
Compare
lmoureaux
force-pushed
the
feature/split-tilespec-cpp
branch
from
1fab1f6 to
1dc71b0
Compare
|
This is the bug that I'm currently having with Trident: [edit: fixed] |
|
Found another bug, this one with amplio2 lakes: [edit: this one is fixed] |
|
Simplify |
This allows to get rid of an idiom where functions would append sprites to an array and return the number of new sprites. The hard-coded maximum of 80 sprites per call no longer exists, which enhances code safety. This is a small step towards breaking down tilespec.cpp, see longturn#430.
The map drawing code will be modularized and layers can ~easily be abstracted away. See longturn#430.
This will allow removing the big switch() in fill_sprite_array(). See longturn#430.
Layers will soon become much more general. Free the name for a new variable. See longturn#430.
This will make them more powerful. See longturn#430.
Using the virtual function will allow to offload drawing code to subclasses. See longturn#430.
This greatly simplifies the definition of ADD_SPRITE in the tileset code and will make it easier to refactor it. See longturn#430.
They are now simple enough to be replaced by direct calls to sprs.emplace_back(). Of the old ADD_SPRITE macros, only ADD_SPRITE_FULL is left. See longturn#430.
The dedicated class layer_background is for now a tiny wrapper around ::fill_sprite_array that works only with LAYER_BACKGROUND. All the code for LAYER_BACKGROUND will be moved to that class in a later commit. See longturn#430.
This commit migrates the code to draw the LAYER_BACKGROUND sprites to the layer_background class. The feature that allowed to hide the terrain was not migrated; see longturn#449. Closes longturn#449. See longturn#430.
Extras with style "Single2" are only drawn on layer "Special2" (they don't have two sprites as was implied by the comments). Also remove some extraneous whitespace.
Instead of duplicating the drawing logic from fill_sprite_array, call fill_sprite_array with a virtual tile that has only the requested extra set. Adjustments were needed to make fill_sprite_array work with this kind of tiles. As a consequence of this change, fill_basic_extra_sprite_array now returns several sprites for shipped tilesets. The help dialog was adjusted to take them all into account, with the limitation that the first sprite dictates the size of the rest and relative offsets are not supported.
lmoureaux
force-pushed
the
feature/split-tilespec-cpp
branch
from
1dc71b0 to
2c3e56b
Compare
lmoureaux
added a commit
to lmoureaux/freeciv21
that referenced
this pull request
Sep 12, 2021
This prevents undefined behavior (in practice, artifacts with Trident). See longturn#450.
lmoureaux
added a commit
to lmoureaux/freeciv21
that referenced
this pull request
Sep 12, 2021
This prevents undefined behavior (in practice, artifacts with Trident). See longturn#450.
lmoureaux
force-pushed
the
feature/split-tilespec-cpp
branch
from
2c3e56b to
8b043b6
Compare
This change allows significant deduplications. The drawing of Single1, Single2 and 3Layers specials is unified: each layer_special may or may not have sprites for each special (which specials is decided when loading the tileset). See longturn#430.
This re-enables the base flags lost in the previous commit.
The same switch(layer) was repeated twice. This was fine when there weren't too many layers, but the amount of duplication grew when adding more. This commit factors out the switch() to a new function.
Straightforward. See longturn#430.
They correspond to two variables declared at the beginning of fill_sprite_array(), do_draw_unit and solid_bg, that are used by several layer types. See longturn#430.
This is a very large one with heavy refactoring. The code was split in smaller functions, boosting readability. Documentation was also improved. No new functionality was added. The unused is_reversed flag was dropped; it was not used in any known tileset and layer_order provides a better alternative. The factorization makes it very straightforward to add more than 3 terrain layers, limited only by the tileset reading code. All shipped tilesets load correctly. See longturn#430.
info.matches_with contains two elements: the first is equal to info.group and the second is the group being matched against. The first element was erroneously used; use the second instead. See longturn#430.
This prevents undefined behavior (in practice, artifacts with Trident). See longturn#450.
lmoureaux
force-pushed
the
feature/split-tilespec-cpp
branch
from
8b043b6 to
2c3a4eb
Compare
Closed
psampathkumar
pushed a commit
to psampathkumar/freeciv21
that referenced
this pull request
Nov 14, 2021
This prevents undefined behavior (in practice, artifacts with Trident). See longturn#450.
lmoureaux
added a commit
to lmoureaux/freeciv21
that referenced
this pull request
May 28, 2023
This is one of the failure modes of bad connections. Fail to connect after 30s (Qt default). Related to longturn#450.
lmoureaux
added a commit
to lmoureaux/freeciv21
that referenced
this pull request
Jun 12, 2023
This is one of the failure modes of bad connections. Fail to connect after 30s (Qt default). Related to longturn#450.
jwrober
pushed a commit
that referenced
this pull request
Jun 17, 2023
This is one of the failure modes of bad connections. Fail to connect after 30s (Qt default). Related to #450.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a major refactoring of the tileset drawing code to split it and simplify what can be. Hopefully at the end we get something much easier to work with.
The following layers have been ported:
LAYER_BACKGROUNDLAYER_SPECIAL*LAYER_BASE_FLAGSLAYER_DARKNESSLAYER_TERRAIN*The following layers have not yet been ported:
LAYER_WATERLAYER_ROADSLAYER_FOGLAYER_UNITLAYER_CITY*LAYER_GRID*LAYER_BASE_FLAGS(which is new)LAYER_OVERLAYSLAYER_TILELABELLAYER_CITYBARLAYER_FOCUS_UNITLAYER_GOTOLAYER_WORKERTASKLAYER_EDITORLAYER_INFRAWORKFeatures dropped:
is_reversedin[tile_*]sections (unused in shipped tilesets, no longer needed since all layers are equivalent)Closes #430.