Nexus, I am honoured that you have had a look through my code and I am grateful that you have taken the time to comment on it and its design.
I'll attempt to address everything that I can.
The 'collection' has a varied range of objects. Not just varied in what they're supposed to be/do but varied in their simplicity. Some are simple and quite limited whereas others can complex and relatively thorough.
You are right about Starfield's limit. This is possibly one of the most limited objects - as you mentioned. I think, though, that it
would be useful in some games (probably more simple or "retro"-looking ones) but less so in games that have very complex space scenes. For example, it could be used in things like Asteroids or side-scrolling space shooters such as Defender or R-Type, or even just as a background for high-scores
It's meant to be a simple and quick way to have this effect in action.
One other point is that is can be any size, so it could be placed in a small sub-display, where space scenes could be too complex to be shown in a lot of detail.
That was the idea behind it anyway.
Crosshair: again you are right about its limits. This is another that is designed for ease of use rather than customisability. I, personally, will be using it for things like designer applications where lining up things are important but I can understand if people just skip this object completely.
I feel differently about ProgressBar. It's true that most UI details are usually custom made to match a style. Progress bars, however, can be rather universal; at least, the more simple ones can. It's quite customisable (like a rectangle shape, for example) and now that I've added anchor for the position of progression, it's easy to draw something at that position. Even if you don't actually
draw the progress bar, the anchors can help with positioning.
Talking of shapes, I originally tried to create Ring as an sf::Shape; that was obviously short-lived as I realised it couldn't create it!
I'm not sure how I feel about API's complexity, especially in complex code. I have been concious of keeping the APIs
relatively complex i.e. the more complex the object, the more flexibility; the more simple the object, the more restrictive.
I actually have a reason that Pie Chart has both radius and diameter methods. I created it based on the overall size - and therefore its diameter, not its radius - so the diameter is the primary method. I added the radius helper method as most people using SFML are used to using radius.
The multiple parameter constructors are kept relatively rare, especially with similar types. Ring's constructor's parameters, however, were based on SFML's circle shape as I wanted it to be similar to use.
Starfield, on the other hand, is meant to be quickly created in a short way. All of its parameters are of different types. I'm not happy with its current range of overloads for regenerate(). I think it needs more!
I am assuming you are referring to Console Screen when you mention create()? I don't think any other object uses one. I'm not sure I understand your point on this. Create in this case (the way I see it) is equivalent to a "resize grid". If you wanted to change the number of cells in the screen, it would be (for example):
consoleScreen.create({40, 20});
With your suggestion of replacing it with move assignment, it would (I believe) look like:
consoleScreen = sw::ConsoleScreen({40, 20});
or have I misunderstood somewhere?
I'm not sure if or-ing enumerators is better than explicit booleans. It may just be seeing it too much during Windows programming that has put me off. I'll have to re-evaluate this.
I suppose that the reference and pointer parameter overloads can be confusing. The main reason behind the null pointer overloads is to allow specifying parameters via reference instead of pointer but also allow nullifying such object. Console Screen's setTexture() is one example. Crosshair allows passing of either a reference or pointer (which can be null and is by default). This is only in the constructor though - not in setWindow(). I think this was an oversight and they should match.
I will have to consider whether or not pointers should also be able to be passed. If not, the pointer parameter should be changed to a null pointer type (
as in Crosshair's setWindow()). If they should be allowed, pointers should be allowed everywhere.
The default values for "enabled" boolean setters, in my view, make more sense grammatically. I think I would personally prefer enable and disable methods, or the method not having "Enabled" in its name, but I used this naming scheme to be closer to how SFML names its methods. I think, if it has enabled in its name, it should be able to enable without having to specifically specifying that it's true that you want it to.
setShowCursorEnabled(); makes sense on its own. You are right, of course; it does seem to be odd without its
setShowCursorDisabled(); counterpart. Having Enabled without Disabled is odd, even in SFML
I think I would prefer one of:
setShowCursorEnabled()
and setShowCursorDisabled(),
setCursorEnabled()
and setCursorDisabled(),
enableShowCursor()
and disableShowCursor(),
enableCursor()
and disableCursor(),
just setShowCursor(bool) without a default parameter
Mainly, if it has enabled, it probably should have disabled. I added enabled because SFML does.
I did consider the Doxygen route. Some reasons came up that caused me to choose otherwise:
- like CMake, I've had trouble with it in the past so I have a very slight irrational hatred for it,
- I don't know if it's possible to update the GitHub wiki automatically from the code, and
- I'm used to "tighter code" during development,
Because of the latter point, and the fact that Selba Ward is the most recent project I have that is on GitHub but not working directly on the repository, I feel maintenance may even be more difficult.
Again, it's something to consider for the future.
virtual void ConsoleScreen::draw(sf::RenderTarget& target, sf::RenderStates states) const; is not a valid declaration.
Fixed.
An embarrassing oversight. Nice catch; thank you!
