2023-10-09 - Swapping implementations
Last week, we implemented a new, more robust and flexible way to define the layout of the hexagonal platforms that compose our gameplay area.
Now, we need to bind the new platform grid to the gameplay, and finally get rid of the old platforms.
I started by reading the "old" code, and removing some dead bits of it.
In the initial PoC implementation, I ended up having two classes, `EnvironmentManagerBhv
` and `HexagonGridInfo
`.
The rest of the game interfaces with `EnvironmentManagerBhv
`, which holds an instance of `HexagonGridInfo
` and queries it for most operations.
We are going to get rid of both, and at the end of the process the rest of the game will interface with our `ZoneManagerBhv
` instead of `EnvironmentManagerBhv
`.
As many of the entities deal with the environment, this is not a super trivial task, but I don't want to change too much code for now, so as first step I'll change `EnvironmentManagerBhv
` so that it uses `ZoneManagerBhv
`, basically acting as an adapter.
When we'll revisit the other entities, along the way, we'll change them so that they directly use the new, clean API that we're going to add to `ZoneManagerBhv
` and `Zone
`, and not the temporary one offered by `EnvironmentManagerBhv
`, designed on the fly during the initial "quick and dirty" prototyping phase.
Ultimately, nothing will use `EnvironmentManagerBhv
` and we'll remove it.
I added to `Zone
` the methods required to feed the requested data to `EnvironmentManagerBhv
`, using temporary implementations for things needing a little more care. In these cases, I usually add a `//TODO
` comment as a reminder.
For example, we're going to define the start platform in the `ZoneDesc
`, and we're going to provide an efficient way to find the closest platform to any point, but for now, an hardcoded value and a brute force will do:
// TODO get start platform from ZoneDesc
public Hex getStartCoords() {
return new Hex(0, 0, 0);
}
// TODO use acceleration structure
public Hex getClosestPlatformToPos(Vector3 vPos) {
Hex? ret = null;
float fMinDist = float.MaxValue;
foreach (var kv in m_rPlatformsMap) {
float fDist = Vector3.SqrMagnitude(vPos - kv.Value.getPos());
if (fDist < fMinDist) {
ret = kv.Key;
fMinDist = fDist;
}
}
return ret.Value;
}
After completing, I tried calling the new implementation from `EnvironmentManagerBhv
`, converting the input/output data so that old and new APIs could communicate.
Unfortunately, when testing the game, the target platform selection (and consequently, the portal system) is broken. It looks like I overlooked something, or introduced a bug.
Tomorrow I'll continue the refactoring and hopefully see what went wrong.
2023-10-10 - Hex directions
Sometimes, instead of making complicated adapters, it can make sense to change a bit the interface exposed by the "old" code that we are going to replace.
It can sound pointless, but it can allow for a smoother transition and can help with debugging.
The first thing I'd like to change is the handling of the hexagonal "direction".
In the PoC, I used a simple `int
` to represent one of the six sides of the hexagons.
So, I had values between `0
` and `5
` acting as identifiers of an hexagon side (and of the direction along that side).
Then, I used some small utility functions like these to do simple operations on those "triangle identifiers":
public static bool areTriOnDiffHalves(int iTriA, int iTriB) {
int iOppositeA = getOppositeTri(iTriA);
return (
iTriB == iOppositeA ||
iTriB == (iOppositeA + 1) % 6 ||
iTriB == (iOppositeA + 5) % 6
);
}
public static int getOppositeTri(int iTri) {
int iTriOpposite = (iTri + 3) % 6;
return iTriOpposite;
}
While acceptable for the PoC, this is quite poor API design and now that we're getting serious, it's time to do better.
Let's take an extract of `Hex
`, that we implemented last week:
//[...]
public struct Hex {
//[...]
// order matters (see rHEX_DIRECTIONS)
public enum EDir {
SE,
NE,
N,
NW,
SW,
S
};
private static readonly Vector3Int[] rHEX_DIRECTIONS =
new Vector3Int[6] {
new Vector3Int( 1, 0, -1), //SE
new Vector3Int( 1, -1, 0), //NE
new Vector3Int( 0, -1, 1), //N
new Vector3Int(-1, 0, 1), //NW
new Vector3Int(-1, 1, 0), //SW
new Vector3Int( 0, 1, -1) //S
};
public Hex getNeighbour(EDir eDir) {
return new Hex(m_vCubeCoords + rHEX_DIRECTIONS[(int)eDir]);
}
//[...]
}
Basically, we defined `Hex.EDir
` to represent the six directions we can move in the grid along the six edges of the hexagon.
So, we can change the old APIs using the `int
`s to represent "hexagonal directions", and have them using a `Hex.EDir
` instead.
There's a little thing that annoys me, tough: the fact that the `EDir
` entries start from "south east", while they should start from "north east" to match our `int
` identifiers.
Is there a specific reason why that's important? Well, it makes the mapping from an angle to an identifier obvious, assuming the canonical representation of the unit circle used in trigonometry.
If you consider any value in `[0...360[
` and do an integer division by `60
`, you get the `[0...5]
` triangle identifier for the slice where that angle falls into.
So, we could add/remove `1
` when we change from the `int
` identifiers to the `Hex.EDir` and vice versa, but as `rHEX_DIRECTIONS
` is only accessed through `getNeighbour
` we might as well change the enumeration so that it matches our identifiers.
We're going to hide this kind of thing behind the API, but now we know that going from the `[0...5]
` index to the `Hex.EDir
` will just need a cast.
public enum eDir {
NE,
N,
NW,
SW,
S,
SE, // moved
};
private static readonly Vector3Int[] rHEX_DIRECTIONS =
new Vector3Int[6] {
new Vector3Int( 1, -1, 0), //NE
new Vector3Int( 0, -1, 1), //N
new Vector3Int(-1, 0, 1), //NW
new Vector3Int(-1, 1, 0), //SW
new Vector3Int( 0, 1, -1), //S
new Vector3Int( 1, 0, -1), //SE, moved
};
This kind of change can look trivial and unnecessary. Like, why bother? Dealing with the `int
` is simple enough. But changing from a generic, primitive type to a specific, meaningful type (even if super-simple, like in the case of the `Hex.EDir
` enumeration) helps in keeping the total cognitive load of the project low.
It's just one tiny detail, but it's one less thing you have to keep in mind.
Also, it makes the API more robust and self-explanatory. Passing a `Hex.EDir
` carries implicitly more information that passing an `int
`, even if behind the scenes it is the same value.
Actually, we could do better than that, using a `byte
` as backing store for the `Hex.EDir
` (so, saving space, 1 byte and not 4) or even specifying that it's a Flags `enum
` where each direction is a bit and we can store in a single byte any subset of the six directions.
[Flags]
public enum EDirs : byte {
None = 0b_0000_0000,
NE = 0b_0000_0001, // 1
N = 0b_0000_0010, // 2
NW = 0b_0000_0100, // 4
SW = 0b_0000_1000, // 8
S = 0b_0001_0000, // 16
SE = 0b_0010_0000, // 32
}
Of course, this would need a different implementation of `getNeighbour
`.
Anyway, keeping this possibility in mind, let's stick to the default, trivial representation for now.
So, I just received my Quest 3, so I'm calling it a day and going to try my new toy.
Tomorrow I'll complete the `int
` -> `Hex.EDir
` transition and hopefully reach the point where I can get rid of the old hexagonal grid.
2023-10-11 - Hex directions refactoring
Before starting with today's development work, a brief note on the Quest 3, that I tried for a bit yesterday: I'd say it's a reasonable step forward from the Quest 2, as one would expect it to be.
The main thing for me is the improved visual clarity (and with wider FOV). I also love that it's less bulky (and consequently, with the better weight distribution, more comfortable).
The color passthrough is not spectacular (there's still a long way to go on that front), but, still a big improvement if we compare it to the greyscale, blurry passthrough offered by the Quest 2. Also, even if the cameras won't change, I think a lot could be improved on the software side to reduce the video distortion. The use of the depth sensor to scan the room and suggest a guardian configuration is very, very cool.
So, we were refactoring stuff to use `Hex.EDir
` and not `int
` when we deal with the six hexagonal edges/directions. In theory, as we modified the enumeration definition so that its values match the `int
` values we used, that should not break anything.
And in fact, as expected, after modifying all the occurrences interested by the change, no visible change in behaviour occurs. That's the essence of refactoring: we improve the code structure without altering its functionalities.
I not only changed `int
` to `Hex.EDir
` where it made sense, but I also isolated all the arithmetic involving those identifiers (that were a bit scattered around the codebase). This way, if we change the representation of directions, we will only need to change the code in one place.
Let's see an example of this kind of change, from `PortalManagerBhv.cs
` (that we'll further discuss when we get there):
public void showIndicators(
Vector2Int vStartCoords,
Hex.EDir dir,
Vector2Int vDestCoords) {
//[...]
//float fDegrees = ((int)dir - 1) * PR_Defs.PI_OVER_3_DEGS; // OLD CODE
float fDegrees = dir.StepCW().GetDegrees(); // NEW CODE
Quaternion qNewPortalRot = Quaternion.Euler(0f, -fDegrees, 0f);
//[...]
}
The old code directly calculated the angle on the basis of the `int
` identifier of the direction.
Now, we call on `dir
` some extension methods doing the same work behind the scenes, but isolated in a `HexExtensions
` class.
public static class HexExtensions {
public static Hex.eDir OppositeCenter(this Hex.eDir dir) {
int iDir = ((int)dir + 3) % 6;
return (Hex.eDir)iDir;
}
public static Hex.eDir OppositeLeft(this Hex.eDir dir) {
int iDir = ((int)dir + 4) % 6;
return (Hex.eDir)iDir;
}
public static Hex.eDir OppositeRight(this Hex.eDir dir) {
int iDir = ((int)dir + 2) % 6;
return (Hex.eDir)iDir;
}
public static Hex.eDir StepCW(this Hex.eDir dir) {
int iDir = ((int)dir + 5) % 6;
return (Hex.eDir)iDir;
}
public static Hex.eDir StepCCW(this Hex.eDir dir) {
int iDir = ((int)dir + 1) % 6;
return (Hex.eDir)iDir;
}
public static float GetDegrees(this Hex.eDir dir) {
float fDegrees = ((int)dir) * PR_Defs.PI_OVER_3_DEGS;
return fDegrees;
}
}
As we can't define static extension methods on an enumeration (which could prove useful), it could make sense to "promote" the enumeration to a simple `HexDir
` struct. But that might be overkill and, anyway, it's not something for today.
We can't stop thinking about every possible detail that could be made better: doing some progress in the bigger scope of the project is also important.
So, for now, we'll stick to the `enum
` and put static methods in the (closely related) `Hex
` struct.
public struct Hex {
//[...]
public static bool AreOnOppositeHalves(eDir a, eDir b) {
return a.OppositeCenter() == b
|| a.OppositeLeft() == b
|| a.OppositeRight() == b;
}
public static EDir RandomDir() {
int iDir = Random.Range(0, 6);
return (Hex.eDir)iDir;
}
//[...]
2023-10-12 - Bug squashing
After completing the `Hex.EDir
` refactoring, I tried again modifying `EnvironmentManagerBhv
` so that it calls `ZoneManagerBhv
` and not the old implementation (based on `HexagonGridInfo
`).
After a bit of visual debugging, I understood the problem.
The order of the entries in `rHEX_DIRECTIONS
` did not match my expectations, that were based on the behaviour of the old implementation.
For example, I expected that accessing the "north east" neighbour of the center platform (the one where the debug lines start, in the following image) I would get the hexagon in that direction considering the top isometric view in Unity (the one with the orange highlight) .
So, I reordered the `rHEX_DIRECTIONS
` entries so that I would get the behaviour I expected.
private static readonly Vector3Int[] rHEX_DIRECTIONS =
new Vector3Int[6] {
new Vector3Int( 1, 0, -1), //NE
new Vector3Int( 0, 1, -1), //N
new Vector3Int(-1, 1, 0), //NW
new Vector3Int(-1, 0, 1), //SW
new Vector3Int( 0, -1, 1), //S
new Vector3Int( 1, -1, 0), //SE
};
After this small change, everything worked flawlessly as with the previous implementation.
Oh, and you know what? `rHEX_DIRECTIONS
` is quite a bad name, it should be more like `rHEXDIR_COORD_OFFSETS
`. Let's change it.
At this point, it's time for the wonderful moment of dead code cleanup.
I usually do a "transitional" commit where there are both new and old implementation, easily switchable.
Then, I remove the old stuff and only leave the new implementation, and do another commit.
If at some point I encounter a bug and I'm not sure if it's related to the new implementation or was already there unnoticed, I can easily switch to the "both implementations" revision and test the old implementation. If the bug was not present, I know that I must only look in the differences brought by the new implementation. Otherwise, it was just a case that had not been considered before. That's usually useful info.
If you want to get fancy with version control, you might instead work on the new implementation in a "feature branch", and merge it to trunk when you're done. I rarely do that kind of thing when working solo, it feels like an overkill. It makes sense when you might need to modify the previous implementation too while you're still working on the new one, of course.
So how did this first clean-up step go?
I'd say pretty well, even if it took a little more than expected. I got rid of a bunch of classes I had thrown around doing the PoC and have replaced a static, hardcoded level layout with one that is easily customizable. Along the way, I globally improved the code introducing the `Hex.EDir
` type in place of `int
`, where appropriate.
Here's a screenshot of the game running with a triangular shaped zone instead of the hexagonal shaped of the PoC.
This only requires this little change in the initialization:
private void initStartupZone() {
IZoneGenerator rStartupZoneGenerator =
//new ProcHexagonalShapedZoneGenerator(3);
new ProcTriangularShapedZoneGenerator(5);
[...]
What could be the next logical step?
I can think of two:
implementing the passage between two zones
adding the possibility to load levels (with arbitrarily placed platforms) from files
I'll decide tomorrow what I feel like doing first.
2023-10-03 - Loading external level data
Ultimately, I decided to implement the `Zone
` loading from external data.
Allowing the player to switch from one zone to another would be more interesting, but would probably take more time, especially if we want to do it properly. And today it's Friday. So, it makes sense to complete something quick and simple today, and start with a more challenging task next week.
So how to store/load a level?
At some point, I plan to try some library which provides very efficient deserialization and offers both a readable/editable representation, and a packed binary one. That's the kind of thing that, AFAIK, should be offered by protobuf. The possibility of doing asynchronous, non-blocking operations will also be critical, as you should never do blocking calls in a VR application's main thread.
But for now, I'll stick to an approach that I've been using the past, and that I know it works reasonably well while being very quick to implement.
I'm going to add to the project these two packages that take care of JSON serialization/deserialization:
`com.unity.nuget.newtonsoft-json`
`jillejr.newtonsoft.json-for-unity.converters`
After importing these packages, we can implement simple methods to save/load to/from JSON asynchronously with very few lines of code.
using System.IO;
using Newtonsoft.Json;
namespace BinaryCharm.ParticularReality.Persistence {
public class PersistenceUtils {
private static readonly JsonSerializerSettings JSON_SER_SETTINGS =
new JsonSerializerSettings {
TypeNameHandling = TypeNameHandling.Auto,
};
private static readonly JsonSerializerSettings JSON_DES_SETTINGS =
new JsonSerializerSettings {
TypeNameHandling = TypeNameHandling.Auto,
MetadataPropertyHandling = MetadataPropertyHandling.ReadAhead
};
private static string serialize<T>(T rData) {
string sRet = JsonConvert.SerializeObject(
rData,
Formatting.Indented,
JSON_SER_SETTINGS
);
return sRet;
}
private static T deserialize<T>(string sJsonData) {
T ret = JsonConvert.DeserializeObject<T>(
sJsonData,
JSON_DES_SETTINGS
);
return ret;
}
public static T fetch<T>(string sFilePath) {
string sJsonText = File.ReadAllText(sFilePath);
T ret = deserialize<T>(sJsonText);
return ret;
}
public static void store<T>(string sFilePath, T data) {
string sJsonText = serialize<T>(data);
File.WriteAllText(sFilePath, sJsonText);
}
}
}
Note that we separated the serialization/deserialization from the file system access (fetch/store).
That's going to be useful when we're going to fetch the level data not from the file system but from some packed resources file.
Is that it?
Not really, to have the serialization and deserialization work properly we have to add some attributes to the data types to be handled.
[...]
public struct ZoneDesc {
[JsonProperty("rPlatforms")]
private readonly PlatformDesc[] m_rPlatforms;
[JsonConstructor]
public ZoneDesc(PlatformDesc[] rPlatforms) {
m_rPlatforms = rPlatforms;
}
[...]
}
[...]
public struct Hex {
[JsonProperty("vCubeCoords")]
private readonly Vector3Int m_vCubeCoords;
[JsonConstructor]
public Hex(Vector3Int vCubeCoords) {
m_vCubeCoords = vCubeCoords;
}
[...]
}
[...]
public struct PlatformDesc {
[JsonProperty("hexCoords")]
public readonly Hex m_hexCoords;
[JsonProperty("ePlatformType")]
public readonly EPlatformType m_ePlatformType;
[JsonConstructor]
public PlatformDesc(Hex hexCoords, EPlatformType ePlatformType) {
m_hexCoords = hexCoords;
m_ePlatformType = ePlatformType;
}
}
Basically, we give to the library the information it needs to properly serialize and deserialize these data types.
When serializing to JSON, the data members will be identified by the names specified in `JsonProperty
`.
e.g, serializing a `
Hex
` entity, `m_vCubeCoords
` will be saved as a `vCubeCoords
` JSON element
When deserializing, those same names will be used to identify the parameter where to pass that data when calling the constructor.
e.g. deserializing a `
Hex
` entity, the data extracted from the `vCubeCoords
` JSON element will be passed as `vCubeCoords
` parameter of the `Hex(Vector3Int vCubeCoords)
` constructor.
These attributes can sometimes be skipped, exploiting some default behaviour, but that make the code less explicit and breaks my coding conventions.
For example, this version should work without attributes:
public struct Hex {
public readonly Vector3Int m_vCubeCoords;
public Hex(Vector3Int m_vCubeCoords) {
this.m_vCubeCoords = m_vCubeCoords;
}
[...]
}
That's possible because:
`
m_vCubeCoords
` is public, and so gets automatically serialized even without the `JsonProperty
` attributethe constructor takes in a parameter that is named exactly like the data member (notice the use of `
this.
` to discriminate)
But considering that we want to keep things tidy, we use the attributes: a small effort to not have names reserved to members (starting with `m_
`) in the JSON data nor as parameter names.
So what does a `ZoneDesc
` serialized to JSON look like?
Here's the very verbose but readable result (abbreviated cutting most platforms, of course).
{
"rPlatforms": [
{
"hexCoords": {
"vCubeCoords": {
"x": -3,
"y": 0,
"z": 3
}
},
"platformType": "normal"
},
{
"hexCoords": {
"vCubeCoords": {
"x": -3,
"y": 1,
"z": 2
}
},
"platformType": "normal"
},
[...] // more platforms here
{
"hexCoords": {
"vCubeCoords": {
"x": 3,
"y": 0,
"z": -3
}
},
"platformType": "normal"
}
]
}
We can easily test the serialization and deserialization by writing to file the generated `ZoneDesc` and then reading back from it:
private void initStartupZone() {
IZoneGenerator rStartupZoneGenerator =
new ProcHexagonalShapedZoneGenerator(3);
//ZoneDesc startupZone = rStartupZoneGenerator.getZone();
ZoneDesc generatedZone = rStartupZoneGenerator.getZone();
string sFilename = "prZoneHex.json";
PersistenceUtils.store(sFilename, generatedZone);
ZoneDesc startupZone = PersistenceUtils.fetch<ZoneDesc>(sFilename);
[...]
}
Instead of initializing the game with the `generatedZone
` obtained by `rStartupZoneGenerator
`, we write it to disk and then read it back to `startupZone
`. And we get the same hexagonal shaped zone we've been using so far.
To make it a little more interesting, now that we have saved the default layout, we can edit it and load the modified version. So, I made a copy (`prZoneHexMod.json
`), removed some platforms from it trying to create an interesting layout, and loaded that as `startupZone
`.
private void initStartupZone() {
string sFilename = "prZoneHexMod.json";
ZoneDesc startupZone = PersistenceUtils.fetch<ZoneDesc>(sFilename);
[...]
}
See you in a week!