Closed grandsong closed 5 years ago
The story as you describe it is pretty accurate. There's a small nuance: even though orientation can only have 2 values, I still think of it as an enum.
If isPointy
is false
, what is it? In other words: if a hex isn't pointy, what is it? This is the main reason I've made orientation
an enum.
But this mainly applies to new users. A quick glance at the docs tells new users a hex can be either pointy or flat, hence a boolean might suffice.
In the next version I'm definitely going to remove isFlat()
. It's the least useful watch (as you call it). Since I'm going to write it in typescript, I think it's a little less weird to keep the enum (and isPointy()
). But maybe I should just have a boolean isPointy
...
I recently came to realize that pointy and flat are just two most common states among all the infinite possible ones of rotation. (There cannot be a third one since there are only two axes, x and y.)
My newest idea is to make the "viewless" grid always pointy, and a gridview can map points in any way that fits.
So, for a visually flat rectangular grid, its logical "height" is actually "width" of the viewless grid.
Theoretically, lib users can rotate in any angle, at any time. Like in the Red Blob Games blog, let end-users to rotate on-the-fly (several of the demos can "switch" between "pointy/flat").
Note that rotation (or any mapping/transformation) of the gridview does not necessarily affect entity views ("pieces") standing on it. The those demoos, hex views (shapes) rotate as the whole grid rotates, while texts remain up-side-up.
Again, the lib had better provide some utilities in order to save lib users from repeating coding of common rotation needs.
If lib users only need flat orientation and feels uncomfortable with the inconsistency between axes of logical grid and of the visual one, they can have "derived" classes FlatGrid and FlatHex.
The only difference is that all x and y are switched before & after all vanilla pointy grid functions. (True, this is a transposition, not a rotation, but such lib users won't care) The best way is to add simple wrappers, not to create painful sub classes. That's why I choose word "derived".
Since you are rewriting the library, I guess it is a good time to rethink over the concepts, principles and architect of the lib, and move on to version 2.0.
This post is the first of my incoming ones.
Let's start with a minor but API-breaking one as an appetizer.
The lib now has three interfaces for the effectively same thing: orientation. This situation is confusing.
This makes me recall my confusion about how to express gender/sex of a person object years ago.
If in a simple world, like in a game, not on Facebook (which provides quite many gender options), there are only two genders: "male" and "female", and
null
is not allowed.Naturally, I could give
Person
a propgender
and let it be"male" | "female"
. Or, I could let it be0 | 1
, but I need to remember which value means what and risk mistakes. Even when I fall back to"male" | "female"
, errors like misspelling, capitalizing ("Male"
) can still happen. Let alone if I need to coopoarate with other team members.The best practice I've come to is prop
isMale
, which is boolean, which is easy-to-use and error-proof.A
Person
class doesn't have to be exactly like a passport.(Well, I am talking about JavaScript only. In C++ or database,
0|1
may be preferred.)Back to your lib.
It is sure that there are only two orientations and
null
is not allowed. So similar to gender.I imagine your story might be like the following:
orientation
.if ... else ...
and as a result you createdisPointy()
.isFlat()
.Now, we have more "freedom" than we need. On the contrary, three options here are as bad as as three watches (to tell the time).
I foresee no need other than
if ... else ...
with orientation in practical uses of this lib.In fact, in your own source code,
orientation
is read only byGrid::rectangle
andGrid::neighborsOf
(when they callcompassToNumberDirection
) besides, of course,isPointy()
andisFlat()
.So, we can safely decide to use one boolean prop, namely
isPointy
, in place of the three.