Now that I have downloaded the code and made some of the modifications I mentioned, I'm finding a few things that I didn't when I just reviewed on GitHub.
1) There are no default constructors for Car and Lake::Float so initializing the vectors containing them causes compilation errors. To resolve, just add a default constructor Ex: Car() = default;
2) I missed the fact that in the Lake constructor, you are using the index of the loop in calculations, which doesn't happen using my suggestion. One of the problems is that you have a class member named 'i' as well, this probably isn't a good name for a class member especially if you are going to be using 'i' as an index in loops.
Here's the updated suggestion:
Code: Select all
Lake::Lake( float yPos )
:
pos( 0.f, yPos ),
boundingBox( pos,
static_cast< float >( Graphics::ScreenWidth ),
static_cast< float >( Graphics::ScreenHeight ) / 3.f ),
floats( numFloats )
{
// The cast is to eliminate the compiler warnings about implicit conversions
const auto iyPos = static_cast< int >( yPos );
// It makes sense to use this style of loop ( as you had before )
// since you are using a counter.
for( int count = 0; count < numFloats; ++count )
{
floats[ count ] = Float( iyPos + ( count * 40 ), 100, 1 );
}
}
I also ran into an issue with variables not being initialized correctly because they were declared "out of order" in the header file.
Original
Code: Select all
class Float
{
RectF boundingBox;
Vec2 pos;
int width;
static constexpr int height = 40;
int direction; //Make random
static constexpr Color color = Colors::MakeRGB(165, 122, 11);
static constexpr int speed = 50;
Graphics& gfx;
public:
Float(int yPos, int width, int direction, Graphics& gfx);
void update(float dt);
void draw();
RectF getBoundingBox();
};
Lake::Float::Float(int yPos, int width, int direction, Graphics& gfx)
:
width(width),
boundingBox(pos, width, height),
gfx(gfx),
direction(direction),
pos( ( direction < 0 ? gfx.ScreenWidth - width : 0 ), yPos )
{
}
Suggested
Code: Select all
class Float
{
int width;
int direction; //Make random
Vec2 pos;
RectF boundingBox;
static constexpr int height = 40;
static constexpr Color color = Colors::MakeRGB( 165u, 122u, 11u );
static constexpr int speed = 50;
public:
Float() = default;
Float( int yPos, int width, int direction );
void update( float dt );
void draw( Graphics &Gfx )const;
RectF getBoundingBox()const;
};
Lake::Float::Float( int yPos, int width, int direction )
:
width( width ),
direction( direction ),
pos( ( direction < 0 ) ?
static_cast<float>( Graphics::ScreenWidth - width ) : 0.f,
static_cast<float>( yPos ) ),
boundingBox( pos,
static_cast<float>( width ),
static_cast<float>( height ) )
{
}
I'm actually kind of surprised that this compiles and runs. Storing a reference I thought makes the class uncopyable, which means it shouldn't be able to go into a vector container. Anyway, what I mean by "out of order" is the variables that you initialize in the constructor's initializer list are initialized in the order they are declared in the header file and not in the order you put them in the initializer list. This means that the boundingBox which relies on pos is being initialized before pos in the original version. Would have been the same for width, but your parameter is also called width with the same letter case, so the compiler used the passed in parameter instead of the class member.