Help me with C++ for my game


#183

Do you have any tips for complex animations?

My original sword slash was 24x24 and used 6 frames, and you’d have to multiply it by 4 to get all directions. I made an alternative that is similar to Link’s Awakening sword animation, but it’s all hardcoded, here’s what the code looks like:

var _facing = facing;

if (_facing == e_dir.right) {

	var _charge_time = 4;
	var _x_offset = 0;
	var _y_offset = -8;
	var _img = 0;
	
	switch (timer) {
		
		// Charging
	    case 0:
			_x_offset = 2;
	        break;
	    case 1:
			_x_offset = 1;
	        break;
	    case 2:
	    case 3:
			_x_offset = 0;
	        break;
		
		// Start slashing
	    case 4:
			_x_offset = 1;
	        break;
	    case 5:
			_x_offset = 4;
	        break;
			
		// Corner slash
	    case 6:
			_img = 1;
			_x_offset = 8;
	        break;
	    case 7:
			_img = 1;
			_x_offset = 8;
	        break;
			
		// Slash to the right
	    case 8:
			_img = 2;
			_y_offset = 0;
			_x_offset = 8;
	        break;
	    case 9:
			_img = 2;
			_y_offset = 5;
			_x_offset = 8;
	        break;
	    case 10:
			_img = 2;
			_y_offset = 4;
			_x_offset = 8;
	        break;
	    case 11:
			_img = 2;
			_y_offset = 4;
			_x_offset = 8;
	        break;
	    case 12:
			_img = 2;
			_y_offset = 4;
			_x_offset = 8;
	        break;
	}
	
	draw_sprite(spr_entity_sword, _img, x+_x_offset, y+_y_offset);

}


if global.timer % 2 == 0 {
	timer++;
}
if 12 < timer {
	change_state(e_state.normal);
}

Here’s the animation https://youtu.be/wm5YnGaX7ms

I don’t mind making a switch statement like this for all directions, I was just curious if there was a better way

EDIT: I only had to add a tiny if statement to make that same code work for the left direction. Now onto up and down.


(Pharap) #184

I take it this code is GML rather than C++?

There’s a fairly easy way to get this to work on the Arduboy whilst potentially saving more memory than using a switch would:

void drawSword(uint8_t x, uint8_t y, Direction direction, uint8_t frame)
{
	struct DrawSwordData
	{
		uint8_t xOffset;
		int8_t yOffset;
		uint8_t frame;
	};

	constexpr uint8_t frameCount = 13;
	
	static const DrawSwordData drawSwordData[directionCount][frameCount] PROGMEM =
	{
		// North
		{
			{ 2, -8, 0 }, { 1, -8, 0 }, { 0, -8, 0 }, { 0, -8, 0 },
			{ 1, -8, 0 }, { 4, -8, 0 }, { 8, -8, 1 }, { 8, -8, 1 },
			{ 8, 0, 2 }, { 8, 5, 2 }, { 8, 4, 2 }, { 8, 4, 2 },
			{ 8, 4, 2 }, 
		},
		// East
		{
			{ 2, -8, 0 }, { 1, -8, 0 }, { 0, -8, 0 }, { 0, -8, 0 },
			{ 1, -8, 0 }, { 4, -8, 0 }, { 8, -8, 1 }, { 8, -8, 1 },
			{ 8, 0, 2 }, { 8, 5, 2 }, { 8, 4, 2 }, { 8, 4, 2 },
			{ 8, 4, 2 }, 
		},
		// South
		{
			{ 2, -8, 0 }, { 1, -8, 0 }, { 0, -8, 0 }, { 0, -8, 0 },
			{ 1, -8, 0 }, { 4, -8, 0 }, { 8, -8, 1 }, { 8, -8, 1 },
			{ 8, 0, 2 }, { 8, 5, 2 }, { 8, 4, 2 }, { 8, 4, 2 },
			{ 8, 4, 2 }, 
		},
		// West
		{
			{ 2, -8, 0 }, { 1, -8, 0 }, { 0, -8, 0 }, { 0, -8, 0 },
			{ 1, -8, 0 }, { 4, -8, 0 }, { 8, -8, 1 }, { 8, -8, 1 },
			{ 8, 0, 2 }, { 8, 5, 2 }, { 8, 4, 2 }, { 8, 4, 2 },
			{ 8, 4, 2 }, 
		},
	};
	
	if(frame < frameCount)
	{
		uint8_t directionIndex = static_cast<uint8_t>(direction);
		const DrawSwordData * drawData = &drawSwordData[directionIndex];
		
		uint8_t xOffset = static_cast<uint8_t>(pgm_read_byte(drawData[frame].xOffset));
		int8_t yOffset = static_cast<int8_t>(pgm_read_byte(drawData[frame].yOffset));
		uint8_t spriteFrame = static_cast<uint8_t>(pgm_read_byte(drawData[frame].frame));
	
		Sprites::drawOverwrite(x + xOffset, y + yOffset, Images::sword, spriteFrame);
	}
}

This assumes Direction is defined as:

enum class Direction : uint8_t
{
	North, East, South, West
};

Any questions?


There might be a way to compress this better,
but I wouldn’t be able to tell without knowing the full picture.

Also this kind of detailed animation is going to eat into your memory budget, so be sure it’s worth it.
(Remember: functionality before polish.)


#185

The images and the mask should cost 96 bytes, I think it’s worth it :D
(but I didn’t take the code itself into consideration)

Also it’s not just for polish, I’ll need enemies to telegraph their attacks (I could still make it fun without it, but it’s gonna be much better with it)

Also, I’ve been working on replacing a few tiles, like my fence that uses 4 tiles. I’ll probably end up with 2 tileset of 16 tiles, with the chunk idea we talked about before - I’ll only need one bit per chunk to store the tileset. I’ll also probably store houses/structures seperately, freeing 4 tiles from the tilesets (unfortunately this means I’ll be drawing a few extra tiles, because they’ll drawn over solid tiles under them)

I’ll also replace my wheat with grass, especially since I’ve discovered it looks almost exactly like Arduventure’s wheat for the map. Anyway here’s the new fence and my trees:

Back to the code, I changed it a bit. Removed a bunch of identical cases. This now handles the right and the left strike

enum e_sword_corner {
	top_right,
	bottom_right,
	top_left,
	bottom_left,
}

var _facing = facing;
var _charge_time = 4;
var _spr = spr_sword_blade;
var _img = 0;
var _x_offset = 0;
var _y_offset = 0;
var _corner = false;

#region left and right
if (_facing == e_dir.right) || (_facing == e_dir.left) {
	_x_offset = 0;
	_y_offset = -8;

	//if timer < _charge_time timer = _charge_time-1;
	switch (timer) {
		
		// Charging
	    case 0:
			_x_offset = 2;
	        break;
	    case 1:
			_x_offset = 1;
	        break;
	    case 2:
	    case 3:
			_x_offset = 0;
	        break;
		
		// Start slashing
	    case 4:
			_x_offset = 1;
	        break;
	    case 5:
			_x_offset = 4;
	        break;
			
		// Corner slash
	    case 6:
			_x_offset = 8;
			_spr = spr_sword_corners;
			_corner = true;
	        break;
			
		// Slash to the right
	    case 7:
			_img = 1;
			_y_offset = 0;
			_x_offset = 8;
	        break;
	    case 8:
			_img = 1;
			_y_offset = 5;
			_x_offset = 8;
	        break;
	    default:
			_img = 1;
			_y_offset = 4;
			_x_offset = 8;
	        break;
	}
	if _facing == e_dir.left {
		_img+=2;
		_x_offset *= -1;
		if _corner {
			_img = e_sword_corner.top_left;
		}
	}
	draw_sprite(_spr, _img, x+_x_offset, y+_y_offset);

}
#endregion



if global.timer % 2 == 0 {
	timer++;
}
if 13 < timer {
	change_state(e_state.normal);
}

How much space would this require vs my switch statement? (I’ll need either one more switch, or two more depending on how I handle top and down sword slashes, but at maximum it’ll be 3)


(Pharap) #186

I can’t predict it without testing.
At minimum, 156 bytes of progmem + the images themselves + whatever the code costs.

Edit:
Just in case you were wondering:
156 = directionCount (4) * frameCount (13) * sizeof(DrawSwordData) (3)

The code itself will be much smaller because it’s simpler.
Essentially the actual functional code is just this bit:

if(frame < frameCount)
{
	uint8_t directionIndex = static_cast<uint8_t>(direction);
	const DrawSwordData * drawData = &drawSwordData[directionIndex];
	
	uint8_t xOffset = static_cast<uint8_t>(pgm_read_byte(drawData[frame].xOffset));
	int8_t yOffset = static_cast<int8_t>(pgm_read_byte(drawData[frame].yOffset));
	uint8_t spriteFrame = static_cast<uint8_t>(pgm_read_byte(drawData[frame].frame));

	Sprites::drawOverwrite(x + xOffset, y + yOffset, Images::sword, spriteFrame);
}

Which is very small and cheap.
It’s one compare, one conditional jump, three progmem fetches, two adds (possibly 2-byte signed adds), some stack pushes and a function call.

Your other code would depend on how much the compiler can figure out about what it can optimise, but I’m happy to bet it would be larger than my code.
The question would be whether or not it’s bigger than the code plus the lookup table.
In general, I find the lookup table approach almost always works out cheaper than a switch,
or in rare cases ends up being the same because the compiler figures out how to turn the switch into a lookup table.


#187

Hey! I took a break from my RPG/Hack 'n Slash and I made a prototype for a platformer that I’m now ready to port to the Arduboy

With your fixed point library, how can I add a custom ‘data type alias’ ?
I’d like to have a type 12 bits for the integer and 4 bits for the fractional part, a UQ12x4

On page 39 of the Arduboy Magazine it says to go look here in your source code to see how it’s done, but I’m not sure how I would actually do it (Would I put it in a header file for my game or do I need to modify the code?)

Also side question, in the Magazine the table shows the range for SQ3x4 to be -15 to +15, shouldn’t it be -7 to +7 because of the bit used for the sign?

Edit: I could simply use UQ16X16 if adding my own is too complex, my game won’t use a lot of RAM so it wouldn’t really matter


(Pharap) #188
using UQ12x4 = UFixed<12, 4>;

I will warn you that there might be a few bugs with the non-standard types, they’re a bit less tested.

If weird things start happening, keep a snapshot for the sake of analysis and then substitute for UQ16x16 (but be warned that will use more RAM and more progmem).

Anywhere before you actually need it would be fine.

https://en.cppreference.com/w/cpp/language/type_alias

Oops. Yes, it should be.
As you can see, I remembered to half all the others.


#189

I’ll have only a player instance so, if worst comes to worst, I think UQ16x16 will do fine. The smallest acceleration is of 1/16 so that’s why it’d be awesome if UQ12x4 ends up working perfectly (but because they’re all multiples of two, it wouldn’t actually make a difference behavior-wise… so it might be my perfectionism leaking again)

#pragma once

#include <Arduino.h>
#include <Arduboy2.h>
#include "FixedPoints.h"

using UQ12x4 = UFixed<12, 4>;

class Player {
    public:
        SQ12x8 x = 0;
        SQ12x8 y = 0;
        uint8_t coins = 0;
};

That’s it? I’m surprised it’s that simple

I think I made a mistake too, it’d be -8 to +8, +8 being exclusive (a normal int8_t goes from -128 to 127, so I think it’d be -8 to (+7 + (15/16))


(Pharap) #190

It would be if you hadn’t written SQ12x8 instead of UQ12x4.
Make sure your type names match.

Otherwise yes.

The whole point of FixedPoints was to make fixed point arithmetic accessible to people who aren’t very experienced, because at the time I’d seen loads of experienced programmers using fixed points and never explaining how they worked, and I felt that less experienced programmers should have that kind of power without having to worry too much about how fixed points actually work, in the same way they can write 0.5f + 0.75f and not have to worry about how IEEE single-precision floating points work.

Technically you could have written UFixed<12, 4> everywhere instead, but I find the shorter names help when you’re using the types a lot.

Also, you should be using angle brackets for FixedPoints.h - it’s actually available on the Arduino library system, so you can use it externally - you don’t need a local copy.

I tried to simplify the grid quite a lot, so some of the numbers are a bit off.

What makes it difficult is that the integer part is signed, but the fractional part isn’t, so -7.5 is actually expressed as -8 + 8/16 and -7.75 is expressed as -8 + 4/16


#191

Nah, there’s an asterisk in the magazine, precising that the upper bound is exclusive. The grid is great :D. I found it very helpful

Oops. Thank you for pointing it out

Honestly this is awesome, thank you. I would have had to mess around quite a bit to get what I needed otherwise. Smooth acceleration is important in a platformer

Other quick question while we’re at it, if I want to have 1/16 in a variable, which are valid ways to do it?

UQ12x8 acceleration = 1/16;
UQ12x8 acceleration = ((UQ12x8)1/(UQ12x8)16);
UQ12x8 acceleration = 1.0/16.0;`
UQ12x8 acceleration = 0.0625;

(I’m still writing a lot of stuff so I couldn’t test the game yet)

Also, other question. I guess I have a lot of questions. How does the fixedRound function work? is it the traditional floor(num + 0.5) or is it “banking rounding”, meaning it rounds towards the nearest even number?

For example, 0.5 would round towards 0, 1.5 would round to 2 with that type of rounding. This way there’s no bias towards bigger numbers (you probably already know all that but I mention it just in case). This only applies for numbers that have the same absolute difference between the number above and below of course. 0.6 would round to 1 normally for example

Edit: here’s how my Player.h is setup:

#pragma once

#include <Arduino.h>
#include <Arduboy2.h>
#include <FixedPoints.h>


enum class PlayerState : uint8_t {
    Walking,
    Running,
    Falling,
    Breaking,
    Dead,
};

using UQ12x4 = UFixed<12, 4>;
using SQ11x4 = SFixed<11, 4>;

class Player {
    public:
        // unsigned
        UQ12x4 x = 0;
        UQ12x4 y = 0;
        UQ12x4 accel = UQ12x4(1)/UQ12x4(16);
        UQ12x4 grav = UQ12x4(1)/UQ12x4(16);
        
        // signed
        SQ11x4 xvel = 0;
        SQ11x4 yvel = 0;
        SQ11x4 maximumWalkingVelocity = 1;
        SQ11x4 maximumRunningVelocity = SQ11x4(1) + SQ11x4(8)/SQ11x4(16);
        SQ11x4 maximumFallingSpeed = SQ11x4(4);
        
        uint8_t coyoteTime = 0;
        uint8_t coins = 0;
        bool onGround = true;
        PlayerState state = PlayerState::Walking;
        PlayerState previousState = PlayerState::Walking;
    private:
        bool isFastEnoughToRun();
        bool isSlowEnoughToWalk();
        void changeState(PlayerState s);
        void collectCoins();
        bool canJump();
};

Other question again, am I gonna get problems adding signed and unsigned variables together?

Ideally unsigned + signed would simply either substract or add the absolute value of the signd integer (I suppose I can use operator overloading if it doesn’t behave like I want?)

I wrote a ton, I hope I’m not using too much of your time. Thank you for your patience


(Pharap) #192

Assuming you mean a fractional part then the following are all valid:

// Implicit casting of float and double
UQ12x4 acceleration = 1.0f / 16.0f;
UQ12x4 acceleration = 1.0 / 16.0;

// Casting
UQ12x8 acceleration = (static_cast<UQ12x8>(1) / static_cast<UQ12x8>(16));

// Constructors
UQ12x8 acceleration = (UQ12x8(1) / UQ12x8(16));
UQ12x8 acceleration = (UQ12x8(1) / 16);
UQ12x8 acceleration = (1 / UQ12x8(16));

// 2-value constructor:
UQ12x8 acceleration = UQ12x8(0, 1);
UQ12x8 acceleration { 0, 1 };

// The 'low level' way
UQ12x4 acceleration = UQ12x4::fromInternal(1);

// Magic constant
UQ12x4 acceleration = UQ12x4::Epsilon;

Epsilon is specifically the smallest possible value that the type can represent, and it’s always equivalent to fromInternal(1).

For unsigned, if the fractional part is >= 0.5 then it uses ceilFixed, otherwise (< 0.5) it uses floorFixed.

For signed values, if the value is positive it behaves like an unsigned number.
If the value is negative it uses floorFixed if the fraction is <= 0.5 and ceilFixed if the fraction is > 0.5.

floorFixed just zeroes the fractional part (i.e. it truncates).
ceilFixed rounds up if the fractional part is greater than 0.

I think this is the ‘half away from zero’ style of rounding, which is how std::round behaves (I’m pretty sure I tried to keep the behaviour as similar as possible to floating point behaviour so it’s easier to substitute).

Always. Even for builtin types.
The compiler issues a warning about this if you attempt it.

You’re best off explicitly casting one of the arguments.
You should consider each case, usually you want to cast to the sign that the result should have, but you might want to cast to a larger type and then cast back down to a small type, or you might need to do some conditional checking (e.g. making sure the signed argument isn’t negative).

You could if you’re going to be doing it a lot,
but be careful it doesn’t end up hiding bugs.


#193

It’s for adding velocity to the position, velocity being signed.

I tried googling for explicit casting and it seems to be all a bit complicated. It sounds like it’d be simpler if I just used a bigger, but signed, type for the player position.

(I want to add my signed xvelocity and yvelocity to my x and y position)

Edit: Yeah I’ll just use SQ19x4 for my x position, and SQ11x4 for my other movement variables. This way I won’t need to mess around with type conversions

Edit: Hmm apparently I need to do an explicit conversion to pass in my x and y to the Sprites drawing functions of Arduboy2.

Apparently I’m doing it wrong. I decided to try uisng getInteger for the fixed point library:
‘getInteger’ was not declared in this scope

Code:

#include <Arduino.h>
#include <Arduboy2.h>
#include <FixedPoints.h>
#include "Player.h"
#include "Images.h"

bool Player::canJump() {
    return (0 < coyoteTime);
}

bool Player::isFastEnoughToRun() {
    return (velWalkMax < absFixed(xvel));
}
bool Player::isSlowEnoughToWalk() {
    return (!isFastEnoughToRun());
}
void Player::draw() {
    int16_t drawx = getInteger(x);
    int16_t drawy = getInteger(y);
    Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);
}

void Player::init(int16_t xpos, int16_t ypos) {
    x = static_cast<SQ19x4>(xpos)
    y = static_cast<SQ11x4>(xpos)
}

(Pharap) #194

Yeah, that can be tricky.

Although now I think about it, isn’t it possible for your player to have negative coordinates?

Example

If you searched ‘explict casting’ you probably ended up at explicit specifier.

It’s not too difficult:

player.x = static_cast<UQ12x4>(static_cast<SQ11x4>(player.x) + player.xvel);
player.y = static_cast<UQ12x4>(static_cast<SQ11x4>(player.y) + player.yvel);

You treat the player position as signed, add the signed value, then case back to unsigned.

Although that could get ugly if your unsigned value is near the upper end…

Probably best to just make your player position signed or use a larger type.

That’ll eat up 32 bits anyway so you might as well just go for SQ27x4.

Although I wonder, do you really need 12 bits for the integer part?

UQ12x4 gets you an integer range of 0-4095.
SQ11x4 gets you an integer range of -2048-2047.

Is your world bigger than 2048?

Yes, conversion to fixed point types are implict,
but conversions from fixed point types are explicit.

This is to prevent bugs and ambiguities.
Few things are worse than finding out that your code is misbehaving because a type that’s supposed to have a fractional part is being implicitly cast to an integer (thus truncating the fractional part).
Bugs like that are the most frustrating to squash.

getInteger is a member function:

int16_t drawx = this->x.getInteger();
int16_t drawy = this->y.getInteger();
Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);

You can also just cast, which is more in keeping with how arithmetic types usually perform:

int16_t drawx = static_cast<int16_t>(this->x);
int16_t drawy = static_cast<int16_t>(this->y);
Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);

If you can afford it, I recommend using roundFixed:

int16_t drawx = static_cast<int16_t>(roundFixed(this->x));
int16_t drawy = static_cast<int16_t>(roundFixed(this->y));
Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);

If in doubt, read the example program:


#195

Hm. I thought I could use 3 bytes only

I guess I calculated wrong, no my levels won’t be bigger than 2000

Nope. Well I thought that maybe yes, but it’s just gonna be void so no it isn’t necessary.

Hm. Makes sense then. It’s a little more work now for a lot less work later

I like my coordinates floored usually (mostly because I’m used to it, but mostly because of how I handle collisions).

wow, so fixed points are an object? This is amazing.
I read a bit about -> before, but I’m not sure why I can’t do just x.getInteger() ?

Hum I’m getting:
invalid static_cast from type 'SQ19x4 {aka SFixed<19u, 4u>}' to type 'int16_t {aka int}'
With this:

int16_t drawx = static_cast<int16_t>(this->x);
int16_t drawy = static_cast<int16_t>(this->y);
Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);

So I went with the this->x.getInteger()


(Pharap) #196

Nope, there’s no uint24_t sadly.

I could write one, but it would have to use assembly to be implemented, so it would be of limited use.

In which case you can get away with SQ11x4 for everything.

If I could change anything about C++, I’d make it so bool and int weren’t implicitly convertible to each other.
The number of subtle bugs that can cause is excruciating.

I’m assuming you mean the OOP definition of ‘object’ rather than the C++ definition (which is slightly different), in which case yes, UFixed and SFixed are template classes.

(Have a read of FixedPoint’s source sometime, your mind will explode at the kind of things C++ can achieve.)

You can do x.getInteger(), I just don’t like eliding the this->.

Why?

Let’s say I take your member function out of context:

void Player::draw() {
    int16_t drawx = x.getInteger();
    int16_t drawy = y.getInteger();
    Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);
}

Pretend you don’t know how Player is defined.
Are x and y member variables or global variables?
You can assume, but it’s impossible to know.

But if you explicitly specify the this->:

void Player::draw() {
    int16_t drawx = this->x.getInteger();
    int16_t drawy = this->y.getInteger();
    Sprites::drawExternalMask(drawx, drawy, spr_player_air, spr_player_air, 0, 0);
}

You now know which variables are member variables, despite the code being taken out of context.

That’s why I always use this->.

I forgot I haven’t added operators for all the integer types.

I’ve stuck an issue on the repo to remind me to figure out why.


#197

Hum. It wouldn’t be that useful anyway, no big deal

I’m a bit confused still, does it mean that through all this:

void Player::update() {
    xvel += input.xInput * accel;
    if (xvel < -velWalkMax) {xvel = -velWalkMax;}
    if (velWalkMax < xvel) {xvel = velWalkMax;}

    if (0 == input.xInput) {
        xvel -= xvelSign() * deceleration;
    }
}

You would put this-> in front of every variable?

By the way I made a sign function for the x and y velocity, because I wasn’t sure how to use your fixedsignbit function

While I’m at it, will this convert the SQ11x4 to int8_t properly?

int8_t Player::velSign(SQ11x4 vel) {
    if (vel < 0) {
        return -1;}
    if (0 < vel) {
        return 1;}
    return 0;
}

Edit: if I do xvel++, will it increment by 1, or by 1/16 ? DoubleEdit: nevermind pretty sure it’s 1


(Pharap) #198

In front of every member variable.

class SomeClass
{
public:
	int x;
	int y;
	
	void moveBy(int deltaX, int deltaY)
	{
		this->x += deltaX;
		this->y += deltaY;
	}
};

It’s based on std::signbit - true if it’s negative, false if it’s positive.

All the free functions are actually implementations of functions that are normally in the standard library.

I wanted to not bother with the ‘fixed’ prefix, so I could just have abs, floor, ceil etc, but because the twits who designed the Arduino library decided to implement them as macros instead of functions I had to use different names to avoid upsetting the compiler.

I wouldn’t really call this ‘converting’ the value because you’re technically using new values (integer literals).

That would work, but the positioning of the curly braces is a bit odd.
You might as well just drop the curly braces completely.

The second comparison is a bit odd as well.
Usually people only put literals on the right hand side of the operator.

I think you’d have to write ++xvel,
I don’t think I implemented the post-increment operator.

It increments by 1 because that’s how it works with floats.
This is one of the few cases where I’d advise using += 1 instead of ++ because it avoids the ambiguity.


#199

I like to put the smallest value to the left always, I find it’s a bit easier to read.

As for (13 == someVariable) it’s to avoid accidentally doing (someVariable = 12) (accidentally changing the value instead of just checking it)

You can’t assign a value to a number, so it just won’t compile if I forget an = sign

I don’t have an explanation for the curly braces, I try to always put them because it’s easy to modify what happens in an if statement and forget that I didn’t put curly braces, making a few lines run always regardless of the condition. I’m open to suggestion for their placements

Good idea

Are there any exceptions?

Edit: I would tend to use a local variable, like int16_t thisX = this->x, and then use thisX throughout the function, like this:

// draw the world
void View::draw() {
    int16_t thisX = this->x;
    int16_t thisY = this->y;
    int8_t xOffset = thisX - (floor(thisX / 8) * 8);
    for(uint8_t i = 0; i < widthInTiles; i++) {
    for(uint8_t j = 0; j < heightInTiles; j++) {
        uint8_t tile = get_tile_progmem(i+thisX*SECTION_WIDTH_IN_TILES, j+thisY*SECTION_HEIGHT_IN_TILES);
        Sprites::drawSelfMasked(i*TILE_WIDTH-xOffset, j*TILE_WIDTH, spr_world, tile);
    }
}

}
(the function isn’t finished yet)

By the way this is amazing. So far the port is coming along great, shouldn’t be long before the game’s ready.


(Pharap) #200

Easier than this?

if (vel < 0)
	return -1;
	
if (vel > 0)
	return 1;
	
return 0;

Or, if you’re happy with ternaries:

return (vel < 0) ? -1 : (vel > 0) ? 1 : 0;

Try reading < as ‘less than’ and > as ‘greater than’.

“If vel is greater than zero” makes more sense than "if zero is less than vel" because vel changes but zero doesn’t.

Also this is a very particular case, because < 0, > 0 and == 0 form a common triad of comparisons.
< 0 essentially means ‘is negative’ and > 0 essentially means ‘is (non-zero) positive’, while == 0 is obviously ‘is zero’.

(Sometimes zero is classed as being neither positive or negative. floats actually have distinct -0 and +0 values. There’s an interesting discussion here. Anyway, I’m going off-piste.)

The key to this is to just forget any maths lessons you’ve had and internalise the idea that == means ‘is equal to’ and = is an assignment.
Think of them as being as different as + (“add”/“plus”) and * (“multiply”/“times”).

When you think about a programming language as being an actual language where the symbols have specific meanings, you’re less prone to making this sort of mistake.
(E.g. read 5 + 6 as “five plus six”, i = 5 as “i equals five” or “assign 5 to i”, if(i == 5) as “if i is equal to five” etc.)

(Oddly enough my first language was Visual Basic, where = actually did mean ‘is equal to’.)

I think the compiler will warn you if you try to assign in an if, but I could be wrong.

Ultimately nothing beats practice and self-discipline.

Out of interest, how many times has this actually happened to you?

I find this is one of those things that people often talk about doing but I rarely see anyone doing it (apart from the Apple goto fail bug, but frankly they shouldn’t have been using goto like that in the first place).

Personally I use Allman style and always elide them if I can.

I like allman style because I like having my braces vertically aligned.
I always look for braces when I’m trying to establish variable scope and when different blocks begin and end.

Basically, when I look at code my brain tries to do this (establish brace pairs):
BracePairs

As well as this (establish block scope):
BlockScope

If I absolutely had to use an alternative style though, I’d probably chose ‘Stroustrup style’ because I hate ‘cuddled elses’.

(I actually got round to sitting down and formalising my code style the other day. It’s still missing a few rules and examples and it doesn’t mention any justification like I had in the original draft, but it’s something at least.)

Exceptions to what?

That’s pretty much redundant.
I’m fairly certain you wouldn’t gain any progmem or speed from doing that


#201

Oh I’m not worried about math lessons, I’m worried about my poor eyes, my terrible attention and my big fingers screwing with everything I type

It’s easy to accidentally write < when it should be >, and then (for me) it’s gonna be hard to find out I made that mistake. Even if it’s in my face I just won’t see it, so for me, always using < makes it easier to read. I know it doesn’t apply to everyone. It’s like having a huge thing tied to my keys so that if I drop them or leave them somewhere, it’s gonna be easier to see. (as a note, on my keyboard < and > are the same button, but > requires Shift)

(0 < variable) and (variable < 0) are so different that I can’t mix them up.

As for == and =, I don’t remember mixing them up, it’s just a precaution for mys-typing adventures. I’m not that worried about this one.

It is very redundant, but it keeps me from writing this-> everywhere I write that x. Which is why my first choice would be to just use x

But I see your point about making the scope obvious and avoiding scope problems. An option to prevent problems if I really don’t want to use this-> could be to add a small g prefix for everything global. Or put all my variables in a global object named global… (feel free to comment on this, I’m just brainstorming)

You’re raising a good question. I don’t remember it happening with a pure function, ever. Only when working with state machines where a lot of it gets developed… through the development of the game. So it does happen, but doing this with pure functions is probably overkill

int8_t Player::velSign(SQ11x4 vel) {
    if (vel < 0)
        return -1;
    if (0 < vel)
        return 1;
    return 0;
}

The only thing I could ever modify in this is the return type and the type being passed in, so I followed our suggestion and removed the brackets

Little edit: I’m using this-> for now because all the other options seem worse


(Pharap) #202

Have you tried using a different colour palette on your editor?

(Assuming you’re not using the Arduino IDE.)

That’s really weird. What keyboard layout does that?

I’d say just use x in that case.
Redundancy is arguably the worse sin (unless it actually has a progmem/performance benefit, which it sometimes does).

Again, that’s arguably worse than just omitting the this->.

(I’m glad you brought that up though, I need to add “do not use hungarian notation” to my style guide.)

C++ actually already has a way of specifying something as being global: prefixing the scope resolution operator.

int local = this->memberVariable + ::global;

You’ve just reminded me of another way to get a compiler error from =.
If your variables are const then assignment is an error:

bool isNegative(const int value)
{
    return (value = 0); // Error: cannot assign to const variable
}

You might grumble at the extra typing to start with,
but you’ll get used to it (probably).

Sometimes “least worse” is the same as “good”. :P