-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace hardcoded tilesets with mod-defined tileset loaders #18728
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
This was referenced Oct 18, 2020
Mailaender
reviewed
Oct 28, 2020
Mailaender
reviewed
Oct 28, 2020
Mailaender
reviewed
Oct 28, 2020
Mailaender
reviewed
Oct 28, 2020
Mailaender
reviewed
Oct 31, 2020
Mailaender
reviewed
Oct 31, 2020
32cf5ff
to
53dc28b
Compare
Fixed, but now depends on #18927. |
Attempting to manage Tiberian Sun's content causes a crash:
|
phrohdoh
previously requested changes
Jan 2, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. |
Rebased. |
Looks good to me and can confirm TS manage content no longer crashes, but needs another rebase. |
Rebased. |
reaperrr
approved these changes
Jan 11, 2021
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 PR implements the long-delayed move from a hardcoded and limiting tileset format to engine-side interfaces + mod-defined parsers, like we have for sequences.
The goal for this PR is to implement appropriately abstracted interfaces without significant changes to logic, so things like merging
Theater
(nowDefaultTileCache
) withTerrainRenderer
or adding support for map-defined tileset overrides are out of scope. This PR is already uncomfortably large without having to consider disruptive changes like these. I expect that we will refine the interfaces and implementation over the next couple of years once mods start trying to use the new capabilities and find the friction points.The main design goal is to remove the hardcoded assumptions that a tileset contains a set of templates that each have a single static sprite for each cell on the map. This allows the engine to support mods that want to use animated tiles (like the C&C remasters), 3D terrain models, or no tiles at all (e.g painting terrain types and having the renderer auto-LAT the transitions, or a single baked terrain image like classic KKnD). For this reason I have named most of the new abstractions around TerrainInfo, which better describes the new basic use.
The main new interfaces classes are:
ITerrainLoader
: This is the terrain equivalent toISpriteSequenceLoader
- the newTerrainFormat:
mod.yaml entry points to a class implementing this interface, which is created to parse the (now arbitrary) tileset yaml.ITerrainInfo
: This is the terrain equivalent toISpriteSequence
, except it makes no assumptions about sprites or artwork. The core engine only needs to know about terrain types and a couple of things for map previews, so this is all it defines.ITemplatedTerrainInfo
: This adds the concept of templates toITerrainInfo
for the benefit of the map editor and bridges/concrete. This is implemented in Mods.Common, and mods that don't want a templated terrain model can happily ignore it if they substitute the default map editor UI logic with their own.ITiledTerrainRenderer
: This interface expands the existingTerrainRenderer
with methods that other traits can use to query template information and sprites without having to harcode references to a specific terrain implementation. The expectation is for mods to provide a matched pair of classes implementing this andITerrainLoader
. The terrain renderer then casts the genericmap.Rules.TerrainInfo
back to its concrete type so it can access the implementation specific information needed to render the terrain.The main testcase here should be obvious: make sure that the existing mods continue to work as expected, focusing on the map editor and things like bridges/concrete that use the new interfaces.
I already have a local branch that implements animated terrain rendering, but that depends heavily on the other open remaster PRs so I expect it will be a while before anyone else can reasonably test it.
Depends on #18701. I suggest reviewing by commit.