Refactoring Map.cs to Enable Mod-defined Map Formats #20582

AspectInteractive2 · 2022-12-24T15:20:35Z

This is a rebased and working version of Ice Reaper's PR #18378, with the addition of splitting out the functionality of Map.cs into optional plug-and-play interfaces. Together this allows the creation of Map formats, which are custom versions of Map.cs that may or may not implement all the existing Map.cs functions (depending on whether they are needed by the mod), and additionally can decide on how these functions are implemented. The most notable example of this currently is isometric vs rectangular cells, these can be defined as two different map formats implementing the same interface, removing the need for numerous 'if else' clauses within the existing Map.cs to check for which C&C mod is currently being used. Additionally this also means that most of the existing functionality in Map.cs can now belong to OpenRA.Mods.Common, which enables users of the mod SDK to modify this behaviour without having to fork the OpenRA repo.

Below are the changes:

A new interface IMap now incorporates all the base functionality of Map.cs
All core functions within OpenRA now point to IMap, rather than the Map class directly
All height/ramp related functions now point to a cast of Map to IMapElevation rather than to Map directly
All Tiles and Resources related functions now point to casts of Map to IMapTiles and IMapResources respectively
DefaultMap and DefaultMapLoader have been created as the initial Map format, and map format loader, however these will eventually be replaced in future PRs with more specialised map formats (e.g. Gen1Map, Gen1MapLoader for TD/RA and Gen2Map, Gen2MapLoader for TS). This implementation sits in OpenRA.Mods.Common rather than OpenRA.Game.

Also please see this comment from pchote for the motivation behind most of the above. The majority of these changes have been implemented in this PR.

NOTE: I have kept my commits unsquashed for now, to make it easier to split out the PR in the future if this becomes necessary.

OpenRA.Game/ModData.cs

OpenRA.Mods.Cnc/Traits/ConyardChronoReturn.cs

OpenRA.Mods.Cnc/Scripting/Properties/ChronosphereProperties.cs

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

OpenRA.Game/Map/MapPreview.cs

OpenRA.Game/Map/Map.cs

PunkPun · 2022-12-25T22:27:35Z

OpenRA.Game/Map/MapPreview.cs

@@ -273,16 +273,17 @@ public MapPreview(Map map, ModData modData)

 			innerData = new InnerData
 			{
-				MapFormat = map.MapFormat,
+				// TO DO: Check if this MapFormat retrieval is correct and working
+				MapFormat = Map.GetMapFormat(Package),


GetMapFormat is only used to get the format before yaml parsing. You are using it after

Fixed, this now uses a cast of IMap as with the other attributes.

Still unresolved. Keep the original implementation

OpenRA.Game/Map/MapGrid.cs

OpenRA.Game/Map/Map.cs

OpenRA.Game/Map/MapPreview.cs

mods/modcontent/mod.yaml

OpenRA.Mods.Common/UtilityCommands/Utilities.cs

OpenRA.Mods.Common/Lint/CheckMapMetadata.cs

OpenRA.Game/Map/Map.cs

PunkPun

Creating a new map crashes

OpenRA.Mods.Common/UtilityCommands/ExtractMapRules.cs

OpenRA.Mods.Common/Traits/World/EditorResourceLayer.cs

OpenRA.Mods.Common/Traits/World/EditorCursorLayer.cs

OpenRA.Mods.Common/Lint/CheckMapMetadata.cs

OpenRA.Mods.Common/EditorBrushes/EditorCopyPasteBrush.cs

OpenRA.Game/World.cs

OpenRA.Game/ModData.cs

AspectInteractive2 · 2022-12-26T16:10:05Z

Creating a new map crashes

This was due to a typo in the If statement for Grid.MaximumTerrainHeight > 0 in DefaultMap.cs, as well as MapSize not being initialised. It now works.

anvilvapre

Change makes sense.

Don't find it a logical choice to keep map implementing all the logic of all interfaces. Better would be to isolate the logic in other objects i.e. Map.GetTileManager().Tiles. But i understand that makes the rewrite even bigger.

Casts are now done without checks. Because the default mods are known to implement them. But to support mods, better would be to try the cast and skip the functionality if not implemented by the mod.

OpenRA.Game/Graphics/Viewport.cs

OpenRA.Game/Graphics/WorldRenderer.cs

anvilvapre · 2022-12-28T18:15:11Z

OpenRA.Game/Map/IMap.cs

+		byte[] SavePreview();
+	}
+
+	public interface IMapElevation : IMap


code above does not check if a map implements this interface. does a direct cast. likely because also maps without elevation implement it(?).

may as well add this to IMap.

Not sure if you referenced the right code above as I don't see the cast. However the idea is that functions that are not implemented would not be called in the first place, because the mod would not call them. E.g.: Elevation isn't used in Red Alert, so it would never call functions that have casts to IMapElevation.

anvilvapre · 2022-12-28T18:17:33Z

OpenRA.Game/Map/MapPreview.cs

@@ -273,16 +273,16 @@ public MapPreview(Map map, ModData modData)

 			innerData = new InnerData
 			{
-				MapFormat = map.MapFormat,
+				MapFormat = ((IMap)map).MapFormat,


unnessary cast? i assume map implements IMap.

Map does not implement IMap, DefaultMap does. Map now only has the core functionality central to all maps, which is basically loading and checking the map, and getting its map format (see the new Map.cs for the remaining functionality of Map, which is now only 164 lines of code.

Rebase fail. The first commit makes a change that is then unmade

This is correct because the IMap interface is first introduced in the second commit. First commit only deals with abstracting the map loader.

OpenRA.Game/Map/MapPreview.cs

OpenRA.Game/Traits/World/ScreenMap.cs

anvilvapre · 2022-12-28T18:29:42Z

OpenRA.Game/World.cs

@@ -189,7 +189,7 @@ internal World(ModData modData, Map map, OrderManager orderManager, WorldType ty
 			this.modData = modData;
 			Type = type;
 			OrderManager = orderManager;
-			Map = map;
+			Map = (IMap)map;


Why not pass an IMap in constructor.

There are two reasons. First World.cs is part of OpenRA.Game, so it is not mod code and therefore may need to use Map.cs without the interface. Secondly it does call some functions that rely on Map not IMap, such as CacheModels, which is out of scope to refactor at this stage. However since World.cs will likely always need to be in OpenRA.Game, there will always be core map functions that it can and will need to access that are not part of IMap, so a cast to IMap while retaining the Map parameter makes the most sense here.

anvilvapre · 2022-12-28T18:33:02Z

OpenRA.Mods.Cnc/Traits/Render/WithBuildingBib.cs

@@ -56,7 +56,7 @@ public IEnumerable<IActorPreview> RenderPreviewSprites(ActorPreviewInitializer i

 				// Some mods may define terrain-specific bibs
 				var sequence = Sequence;
-				if (map.Tiles.Contains(cell))
+				if (((IMapTiles)map).Tiles.Contains(cell))


to me it seems more logical to let a map own a TileManager. i.e. map.GetTileManager(). promoting splitting logic.

i.e. otherwise you could have a map implement endless interfaces.

Map no longer has Tiles, this functionality now belongs to IMapTiles and the current implementation sits in DefaultMap rather than Map. And Tiles is a CellLayer class which seems to work the same as your proposed TileManager class. In any case changing how Tiles are managed is not the goal of this PR. All I am doing here is moving the functionality out of Map.cs and into DefaultMap, behind an interface IMapTiles.

i understand. but then still, it does not make sense to have IMapTiles inherit from IMap. Completely different things. Like letting ITrait inherit from IWidget. just because some class will implement both.

but i understand how you came to this solution.

I agree, but this is how it was done with the Tilesets refactoring as well (see below link), with ITemplatedTerrainInfo and ITerrainInfoNotifyMapCreated both inheriting from ITerrainInfo. The downside is it adds a dependency to IMap where it may not need to be there, but on the plus side it means less casting is necessary, since you can cast to IMapTiles and still use the functionality from IMap, rather than having to cast twice.

https://github.com/OpenRA/OpenRA/pull/18728/files#diff-b08312ffb1592016a1b6797006dca462f3eb0f22a38ca2956e55e1e9c4f49ee1R14-R23

anvilvapre · 2022-12-28T18:36:25Z

OpenRA.Mods.Cnc/Traits/World/TSEditorResourceLayer.cs

@@ -42,6 +42,8 @@ public TSEditorResourceLayer(Actor self, TSEditorResourceLayerInfo info)

 		bool IsValidVeinNeighbour(CPos cell, CPos neighbour)
 		{
+			var mapRamp = ((IMapElevation)Map).Ramp;


try the cast, to allows mods to keep using this logic, even if disabled.

Not sure what you mean here. Mods can keep using this logic if they want and it is still part of the default functionality found in DefaultMap.

Likewise here.

OpenRA.Mods.Cnc/Traits/World/TSEditorResourceLayer.cs

PunkPun

Reviewed the first commit. Running make throws tons of errors, and the commit should be renamed to something like: "Abstracted map loading to DefaultMapLoader : IMapLoader"

OpenRA.Mods.Common/Widgets/Logic/Editor/NewMapLogic.cs

OpenRA.Mods.Common/UtilityCommands/ExtractMapRules.cs

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

PunkPun · 2023-02-01T15:37:30Z

OpenRA.Game/Map/MapPreview.cs

@@ -273,16 +273,16 @@ public MapPreview(Map map, ModData modData)

 			innerData = new InnerData
 			{
-				MapFormat = map.MapFormat,
+				MapFormat = ((IMap)map).MapFormat,


Rebase fail. The first commit makes a change that is then unmade

PunkPun · 2023-02-01T15:42:15Z

OpenRA.Game/Map/Map.cs

 			// Empty rules that can be added to by the importers.
 			// Will be dropped on save if nothing is added to it
 			RuleDefinitions = new MiniYaml("");
-
-			Tiles = new CellLayer<TerrainTile>(Grid.Type, size);


Rebase fail. First commit comments this out

PunkPun · 2023-02-02T20:14:26Z

It's weird how you added a massive interface of IMapCell.cs, even adding a new file in third commit and then removed it in 6th commit. There are a bunch of errors in the first commits that then get fixed in following commits, even when they couldn't have not been introduced in the first place

AspectInteractive2 · 2023-02-04T06:54:28Z

It's weird how you added a massive interface of IMapCell.cs, even adding a new file in third commit and then removed it in 6th commit. There are a bunch of errors in the first commits that then get fixed in following commits, even when they couldn't have not been introduced in the first place

The purpose of keeping the commit is purely to show the genesis of the project, since it went through a revision of separating out the IMapCell interface, before merging it back into IMap after discussing it and finding that the cell logic is too closely coupled to the engine to be isolated at this stage. The first and last commits are the most meaningful, since the first shows the initial loader and map format design, while the latter shows the new interface that is being implemented to support the new map format. All of the commits in between are purely revisions to show the evolution, in case you are wondering how the various interfaces came to be.

PunkPun · 2023-02-04T07:34:42Z

How this PR came to be or it's evolution is irrelevant and just noise making it harder to find regressions

AspectInteractive2 · 2023-02-04T07:42:39Z

How this PR came to be or it's evolution is irrelevant and just noise making it harder to find regressions

Sure, okay, I've squashed the commits and renamed the first commit.

PunkPun · 2023-02-07T15:19:21Z

OpenRA.Game/Support/ExceptionHandler.cs

@@ -38,7 +38,7 @@ public static void HandleFatalError(Exception ex)
 			if (Game.OrderManager != null && Game.OrderManager.World != null && Game.OrderManager.World.Map != null)
 			{
 				var map = Game.OrderManager.World.Map;
-				Log.Write("exception", $"on map {map.Uid} ({map.Title} by {map.Author}).");
+				Log.Write("exception", $"on map {((Map)map).Uid} ({((Map)map).Title} by {((Map)map).Author}).");


casts could be moved higher up

OpenRA.Mods.Cnc/UtilityCommands/ImportTSMapCommand.cs

PunkPun · 2023-02-07T15:29:00Z

OpenRA.Mods.Common/EditorBrushes/EditorCopyPasteBrush.cs

-			var mapResources = worldRenderer.World.Map.Resources;
+			var map = worldRenderer.World.Map;
+			var gridType = map.Grid.Type;
+			var mapResource = (IMapResource)worldRenderer.World.Map;


Suggested change

var mapResource = (IMapResource)worldRenderer.World.Map;

var mapResource = (IMapResource)map;

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

OpenRA.Mods.Common/Traits/World/ElevatedBridgeLayer.cs

PunkPun · 2023-02-07T15:49:11Z

OpenRA.Mods.Common/Traits/World/LegacyBridgeLayer.cs

@@ -67,12 +69,15 @@ public void WorldLoaded(World w, WorldRenderer wr)

 		void ConvertBridgeToActor(World w, CPos cell)
 		{
+			var map = w.Map;


this variable isn't needed

OpenRA.Mods.Common/UtilityCommands/ExtractMapRules.cs

mods/modcontent/mod.yaml

PunkPun · 2023-04-06T13:11:38Z

OpenRA.Game/Map/Map.cs

+		Map Load(ModData modData, IReadOnlyPackage package);
+		Map Create(ModData modData, ITerrainInfo terrainInfo, int width, int height);
+		string ComputeUID(ModData modData, IReadOnlyPackage package);
+		void UpdatePreview(ModData modData, MapPreview mapPreview, IReadOnlyPackage p, IReadOnlyPackage parent, MapClassification classification, string[] mapCompatibility, MapGridType gridType);


couldn't we just pass ModData in the IMapLoader constructor and avoid passing it in every function? MapPreview also contains a reference to MapData so it's unnecessary to pass

PunkPun · 2023-04-06T13:27:13Z

OpenRA.Game/Map/Map.cs

+#pragma warning disable IDE1006 // Naming Styles
+		protected readonly ModData modData;
+		protected Translation Translation;
+#pragma warning restore IDE1006 // Naming Styles


Just follow the rule. There are only a few uses of these variables anyway

From memory following this rule actually isn't possible because modData must be distinct from ModData, yet the naming rule does not allow lowercase 'm'. So the only way to follow the rule is to do something like ModDataInstance, which would be nastier than disabling the rule for those two lines imo.

PunkPun · 2023-04-06T13:35:37Z

OpenRA.Game/Map/IMap.cs

Perhaps rename file to MapInterfaces and move IMapLoader here

PunkPun · 2023-04-06T13:46:45Z

OpenRA.Game/Map/Map.cs

-		}
-
-		static int GetMapFormat(IReadOnlyPackage p)
+		public static int GetMapFormat(IReadOnlyPackage p)


It's weird to have this defined in Map.cs when it is only used in DefaultMap for uid extraction

PunkPun · 2023-04-06T13:51:28Z

OpenRA.Game/Map/MapPreview.cs

@@ -273,16 +273,17 @@ public MapPreview(Map map, ModData modData)

 			innerData = new InnerData
 			{
-				MapFormat = map.MapFormat,
+				// TO DO: Check if this MapFormat retrieval is correct and working
+				MapFormat = Map.GetMapFormat(Package),


Still unresolved. Keep the original implementation

PunkPun · 2023-04-06T13:54:21Z

OpenRA.Game/Map/MapPreview.cs

@@ -63,8 +63,8 @@ public class RemoteMapData

 	public class MapPreview : IDisposable, IReadOnlyFileSystem
 	{
-		/// <summary>Wrapper that enables map data to be replaced in an atomic fashion.</summary>
-		class InnerData
+		/// <summary>Wrapper that enables map data to be replaced in an atomic fashion</summary>


PunkPun · 2023-04-06T13:55:12Z

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

+			new MapField("Voices", "VoiceDefinitions", required: false),
+			new MapField("Music", "MusicDefinitions", required: false),
+			new MapField("Notifications", "NotificationDefinitions", required: false),
+			//new MapField("Translations", "TranslationDefinitions", required: false)


Rebase error

PunkPun · 2023-04-06T14:02:53Z

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

+
+			if (yaml.TryGetValue("MapFormat", out var temp))
+			{
+				var format = FieldLoader.GetValue<int>("MapFormat", temp.Value);


newData.MapFormat is never set

penev92 · 2023-04-06T14:52:57Z

OpenRA.Game/Map/IMap.cs

+		SequenceSet Sequences { get; }
+		CellRegion AllCells { get; }
+		List<CPos> AllEdgeCells { get; }
+		WDist CellHeightStep { get; }


Why isn't this on the Elevation interface?

#20582 (comment)

PunkPun · 2023-04-06T14:42:39Z

OpenRA.Mods.Common/UtilityCommands/CheckYaml.cs

@@ -97,7 +97,7 @@ void IUtilityCommand.Run(Utility utility, string[] args)
 					if (package == null)
 						continue;

-					using (var testMap = new Map(modData, package))
+					var testMap = modData.MapLoader.Load(modData, package);


Keep the using

PunkPun · 2023-04-06T14:50:15Z

OpenRA.Game/Map/Map.cs

-			else
-				throw new InvalidDataException($"Unknown binary map format '{Format}'");
-		}
-	}



"An opening brace should not be followed by a blank line"

PunkPun · 2023-04-06T14:53:44Z

OpenRA.Mods.Common/Lint/CheckMapMetadata.cs

@@ -11,25 +11,28 @@

 using System;
 using OpenRA.Server;
+using OpenRA.Mods.Common.MapFormats;


"Using directives should be ordered alphabetically by the namespaces"

PunkPun · 2023-04-06T14:56:08Z

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

+	{
+		public const int SupportedMapFormat = 12;
+
+		/// <summary>Defines the order of the fields in map.yaml</summary>


"Documentation text should end with a period"

PunkPun · 2023-04-06T14:59:07Z

OpenRA.Mods.Common/MapFormats/DefaultMap.cs

+			catch (Exception e)
+			{
+				Log.Write("debug", "Failed to load rules for {0} with error {1}", Title, e);
+				rules = Ruleset.LoadDefaultsForTileSet(modData, Tileset);


"Unnecessary assignment of a value to 'rules'"

Something went really wrong here. rules is set but not used

penev92 · 2023-04-06T15:03:04Z

OpenRA.Mods.Common/Traits/World/EditorResourceLayer.cs

@@ -106,7 +106,7 @@ public EditorResourceLayer(Actor self, EditorResourceLayerInfo info)
 				kv => kv.Value.ResourceIndex,
 				kv => kv.Key);

-			Map.Resources.CellEntryChanged += UpdateCell;
+			((IMapResource)Map).Resources.CellEntryChanged += UpdateCell;


A few lines above don't do Map = self.World.Map;. You have no idea whether that would give you a map implementation you can use. Check the type and only use if it implements IMapResource. Otherwise you are free to throw here so the modder knows not to use this trait with an incompatible map format.
Then remove all the casts like this one.
Repeat that for every affected trait in this commit.

PunkPun · 2023-04-06T15:37:04Z

The extraction of UpdateFromMap from MapPreview to DefaultMap doesn't make any sense. If you wanted customisability you'd need to extract way way more. Atm it's just a redundant change. Ping @IceReaper

PunkPun · 2023-04-06T15:51:00Z

I'd say MapPreview is completely out of scope of this PR

pchote

In general (and i'm pretty sure I have already explained this more than once!) this is very difficult to review and has a high risk of regressions because it is attempting to do things backwards and touching a lot of things twice.

Like I showed with the tileset generalisation, the simpler approach is to start by introducing the interfaces, changing the way that world.Map is accessed without changing anything about how that object is created. This separates out concerns allowing each interface to be done with its own commit.

Only at the end do we try to replace Map with DefaultMap etc - none of the users of world.Map need to change at this point because they are all already using the interfaces.

A very partial specific review below.

pchote · 2023-04-07T13:18:06Z

OpenRA.Game/Graphics/TerrainSpriteLayer.cs

@@ -32,7 +32,7 @@ public sealed class TerrainSpriteLayer : IDisposable
 		readonly bool restrictToBounds;

 		readonly WorldRenderer worldRenderer;
-		readonly Map map;
+		readonly IMap map;


The TerrainSpriteLayer constructor should be checking that world.Map implements IMapTiles and throw an exception if it doesn't (see how the tileset code does this for examples).

pchote · 2023-04-07T13:19:25Z

OpenRA.Game/Graphics/TerrainSpriteLayer.cs

@@ -88,7 +88,7 @@ public void Update(CPos cell, Sprite sprite, PaletteReference palette, float sca
 			var xyz = float3.Zero;
 			if (sprite != null)
 			{
-				var cellOrigin = map.CenterOfCell(cell) - new WVec(0, 0, map.Grid.Ramps[map.Ramp[cell]].CenterHeightOffset);
+				var cellOrigin = map.CenterOfCell(cell) - new WVec(0, 0, map.Grid.Ramps[((IMapElevation)map).Ramp[cell]].CenterHeightOffset);


var cellOrigin = map.CenterOfCell(cell); if (map is IMapElevation mapElevation) cellOrigin -= new WVec(0, 0, map.Grid.Ramps[mapElevation.Ramp[cell]].CenterHeightOffset);

pchote · 2023-04-07T13:21:00Z

OpenRA.Game/Graphics/Viewport.cs

@@ -242,6 +242,7 @@ public CPos ViewToWorld(int2 view)
 		{
 			var world = worldRenderer.Viewport.ViewToWorldPx(view);
 			var map = worldRenderer.World.Map;
+			var mapRamp = ((IMapElevation)map).Ramp;


Likewise here, Viewport should be able to handle maps that don't implement IMapElevation (this is one of the main usecases in the upstream mods for these interfaces!)

pchote · 2023-04-07T13:23:47Z

OpenRA.Game/Map/MapGrid.cs

@@ -126,7 +126,7 @@ public class MapGrid : IGlobalModData

 		public CellRamp[] Ramps { get; }

-		internal readonly CVec[][] TilesByDistance;
+		public readonly CVec[][] TilesByDistance;


This is only used within DefaultMap. Move it there instead of making it public.

pchote · 2023-04-07T13:26:22Z

OpenRA.Game/Map/ProjectedCellRegion.cs

@@ -45,7 +45,7 @@ public ProjectedCellRegion(Map map, PPos topLeft, PPos bottomRight)
 			var heightOffset = map.Grid.Type == MapGridType.RectangularIsometric ? maxHeight : maxHeight / 2;

 			// Use the map Height data array to clamp the bottom coordinate so it doesn't overflow the map
-			mapBottomRight = map.Height.Clamp(new MPos(bottomRight.U, bottomRight.V + heightOffset));
+			mapBottomRight = ((IMapElevation)map).Height.Clamp(new MPos(bottomRight.U, bottomRight.V + heightOffset));


This should support maps that don't implement IMapElevation.

pchote · 2023-04-07T13:33:36Z

OpenRA.Mods.Cnc/Traits/World/JumpjetActorLayer.cs

@@ -56,7 +56,7 @@ public JumpjetActorLayer(Actor self, JumpjetActorLayerInfo info)
 							continue;

 						neighbourCount++;
-						neighbourHeight += map.Height[neighbour];
+						neighbourHeight += ((IMapElevation)map).Height[neighbour];


Check that map implements the interface first and throw if it doesn't (see tileset code).

pchote · 2023-04-07T13:33:52Z

OpenRA.Mods.Cnc/Traits/World/TSEditorResourceLayer.cs

@@ -42,6 +42,8 @@ public TSEditorResourceLayer(Actor self, TSEditorResourceLayerInfo info)

 		bool IsValidVeinNeighbour(CPos cell, CPos neighbour)
 		{
+			var mapRamp = ((IMapElevation)Map).Ramp;


Likewise here.

…s, and IMapElevation

abcdefg30 · 2023-04-22T17:58:16Z

Sorry, but I have to agree with @pchote here. I don't think this PR can be merged in it's current state, but should be split into more digestible parts.

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 2 times, most recently from d22ecab to 18fdf40 Compare December 25, 2022 07:38

PunkPun reviewed Dec 25, 2022

View reviewed changes

OpenRA.Game/Map/MapGrid.cs Outdated Show resolved Hide resolved

OpenRA.Game/Map/Map.cs Show resolved Hide resolved

PunkPun reviewed Dec 26, 2022

View reviewed changes

AspectInteractive2 force-pushed the mod-mapformats-rebased branch from fcba3fa to ff70c18 Compare December 26, 2022 16:21

anvilvapre reviewed Dec 28, 2022

View reviewed changes

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 4 times, most recently from af75344 to 7325fb1 Compare January 2, 2023 13:55

abcdefg30 mentioned this pull request Jan 3, 2023

Move map format loading and saving into mods. #18378

Closed

AspectInteractive2 mentioned this pull request Jan 23, 2023

Overhaul sequence parsing and sprite loading #20597

Merged

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 3 times, most recently from 892f19a to 1b179ee Compare January 30, 2023 02:30

PunkPun reviewed Feb 1, 2023

View reviewed changes

PunkPun added the PR: Fixup requested label Feb 2, 2023

AspectInteractive2 force-pushed the mod-mapformats-rebased branch from 6403514 to 3548cdf Compare February 4, 2023 06:57

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 2 times, most recently from 68f7759 to 2b39b3d Compare February 4, 2023 07:41

PunkPun reviewed Feb 7, 2023

View reviewed changes

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 2 times, most recently from 97f83a1 to 642df58 Compare March 11, 2023 14:11

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 7 times, most recently from 9c1d13a to 65621cd Compare March 18, 2023 05:18

AspectInteractive2 force-pushed the mod-mapformats-rebased branch 7 times, most recently from 70bc065 to 65103d9 Compare April 6, 2023 13:03

PunkPun reviewed Apr 6, 2023

View reviewed changes

penev92 reviewed Apr 6, 2023

View reviewed changes

PunkPun reviewed Apr 6, 2023

View reviewed changes

penev92 reviewed Apr 6, 2023

View reviewed changes

pchote reviewed Apr 7, 2023

View reviewed changes

Abstracted map loading to DefaultMapLoader : IMapLoader

c34903e

AspectInteractive2 force-pushed the mod-mapformats-rebased branch from 65103d9 to cdde97b Compare April 22, 2023 06:32

Implementation of map format interfaces IMap, IMapTiles, IMapResource…

6d42063

…s, and IMapElevation

AspectInteractive2 force-pushed the mod-mapformats-rebased branch from b96d009 to 6d42063 Compare April 22, 2023 09:49

abcdefg30 added the PR: Conceptual concerns label Apr 22, 2023

PunkPun mentioned this pull request Apr 25, 2023

Add IMapCredentials #20826

Draft

	var mapResource = (IMapResource)worldRenderer.World.Map;
	var mapResource = (IMapResource)map;

Refactoring Map.cs to Enable Mod-defined Map Formats #20582

Are you sure you want to change the base?

Refactoring Map.cs to Enable Mod-defined Map Formats #20582

Conversation

AspectInteractive2 commented Dec 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PunkPun left a comment

Choose a reason for hiding this comment

AspectInteractive2 commented Dec 26, 2022

anvilvapre left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AspectInteractive2 Dec 29, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PunkPun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PunkPun Feb 1, 2023 • edited

Choose a reason for hiding this comment

PunkPun commented Feb 2, 2023

AspectInteractive2 commented Feb 4, 2023 • edited

PunkPun commented Feb 4, 2023

AspectInteractive2 commented Feb 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penev92 Apr 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

penev92 Apr 6, 2023 • edited

Choose a reason for hiding this comment

PunkPun commented Apr 6, 2023

PunkPun commented Apr 6, 2023

pchote left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abcdefg30 commented Apr 22, 2023

AspectInteractive2 commented Dec 24, 2022 •

edited

AspectInteractive2 Dec 29, 2022 •

edited

PunkPun Feb 1, 2023 •

edited

AspectInteractive2 commented Feb 4, 2023 •

edited

penev92 Apr 6, 2023 •

edited

penev92 Apr 6, 2023 •

edited

pchote left a comment •

edited