Flapping Shooter Feedback & Tips

If I might make a suggestion, what about “Koumori Caverns”?

Koumori is Japanse for ‘bat’: https://jisho.org/word/蝙蝠
(Chiroptera, not cricket. :P)

1 Like

Yes.   

1 Like

Well, I have collisions… sort of.

While the game detects collisions between bullets and enemies, it seems to do some strange things.
Sometimes, it works as expected. Other times, it seems delayed or to not detect a collision at all. In still other cases, collisions are detected where there should be none.

It is difficult for me to determine the cause of this, especially because I cannot really isolate the collision system from the rest of the game. Regardless, I am updating the demo on the first post with this updated version. Maybe this issue will be familiar to someone offhand.

I completely broke the code in an attempt to clean it up, so I will hold off posting the code until I fix that.

Note: I have only implemented enemy/bullet collisions.

Is your code hosted on GitHub or elsewhere?

Are you using the ‘pixel-based’ collision from my code or standard collision detection?

I am using your pixel based collision.
I don’t have the code on github yet. I was trying to make it as reader-friendly as possible before posting it.

Further testing revealed that collision is always detected if the bullet is above the enemy, regardless of distance.

I will post the code and add a link tomorrow evening.

Don’t worry too much about that, that’s what branches are for.
You can shove all the messy in-development code onto a ‘develop’ branch while it’s being developed,
then when it’s ready you can clean it up before commiting to the ‘master’ branch.

https://help.github.com/en/desktop/contributing-to-projects/creating-a-branch-for-your-work


Failing that, you could just shove it in a gist:
https://help.github.com/en/github/writing-on-github/creating-gists

Messy code it is: https://github.com/Quickdrawkid/BAT-GAME/tree/master
Unclean! Unclean!

MOST of the variables should be made into constants or local variables. I did this on the “clean” version, but somehow broke it.

I am not really familiar with github, so let me know if I need to fix a setting or something. I did make a “develop” branch, though.

I really appreciate the assistance.

1 Like

I’ve seen a lot worse. (A lot worse.)

My biggest gripe is that you’re using macros.
The first thing you should do when you have chance is to scrap the macros and use constexpr variables instead.
(And yes, you can use constexpr variables to specify the size of an array.)

The second thing you should do is move your images to a separate header file,
just for the sake of decluttering.
Images tend to be autogenerated as they’re not something you’d want to be looking at often so usually they get shoved in a header file and then ignored until new images are needed.

Eventually you should end up moving more of your code into separate headers so you have several short files instead of one long one,
but at the moment you’re not quite at the point where you should be panicking about it*.

My other gripes are more minor, mainly to do with inconsistency and formatting.
I’d offer work on the formatting, but I don’t know if you’d be happy with the formatting style I’d offer.
Do you have a preference when it comes to brace style and tabs vs spaces?
(Personally I’m strongly in favour of Allman style and tabs.)

Nor was I when I first started visiting the forum.

It takes some time to get used to, particularly because it’s based on git, git is horribly complicated, and a lot of tutorials for it assume you’re using command line git with it,
but I’ve found that GitHub’s interface is actually easier to work with than git, you just have to find the right info.


* When should you start panicking?

As a rough guide:

  • 0…250 lines per file is ideal
  • 250…500 lines is mildly concerning, but not a major issue
  • at 500…750 lines you should really consider refactoring
  • at 750…1000 lines you have a problem
  • at 1000 lines or more I phone the code police and they ship you off to gitmo :P
1 Like

Allman style is actually what we were taught in college, so I dont see anything wrong with that.

The macros will be done away with when I get a chance.

1 Like

I approve of your college. :P

Before I learnt C++ I mainly used C#, where Allman style is more predominant.
However, I’ve tried other styles and I’ve decided that Allman style has more practical merits.

(I actually learnt C# during my college days, but it’s not what they taught us.
They taught us in Visual Basic, and I took up C# in my spare time.)

I’ve made some commits on a local fork that do away with the macros and move the images into a separate header:

If you’d like, I can make a PR (pull request) and you can merge the commits onto the development branch.


Completely unrelated, but I’ve been wondering:
what is your forum icon supposed to be?

No matter how much I stare at it, I can’t figure it out.

Merging the changes is okay with me. I assume I would need to approve changes?


My icon is Destructosaurus. He’s a giant, 6 legged, fire breathing dinosaur. He is the OG kaiju in my little fantasy world. See related pic of my cubicle, bottom left. (The icon is a 16 x 16 sprite for the forbidden future project)

2 Likes

Indeed you would. (Hence why it’s a ‘pull request’.)

Unfortunately GitHub seems to be having issues at the moment.

I’m assuming it’s actually GitHub having the problems and not just me?
Maybe the latest Firefox update broke AJAX or something?

I know what a 怪獣 is, but you lost me with ‘OG’.

I.e. another person attempting a Pokemon-like.
(Or possibly a Digimon-like.)

OG is “original gangster.” Basically, he’s the first and exceptional.

It is not a Pokemon like game, though. It is basically a pacman-esque game where you can destroy parts of the maze to open new paths, along with some other mechanics.

1 Like

Forgive the delay, I’ve fixed my GitHub problem.
(Somedays boredom leads me to do stupid things in the name of science…)

Here’s the PR at last:

I was in such a hurry to get it done that I forgot to retarget it to ‘develop’ instead of ‘master’,
but since there aren’t any other commits in the way,
that can be fixed easily enough by just deleting ‘develop’ and recreating it after the PR has been merged.

Thank you @Pharap for the improvements! I have merged the changes.

I sat down and played with this a bit more this evening with the intent of fixing the collisions. The only thing I could think to change is how the Collide() functions are called.

So here is the code I was using:

bool collisionSonic (int enemyX, int enemyY, unsigned char PROGMEM enemySprite) { //adapted from filmote
    
  for (uint8_t i = 0; i < maxSonics; i++) {

    if (sonic[i].enabled == true) {

      if (collide(enemyX, enemyY - getImageHeight(enemySprite), enemySprite, sonic[i].x, sonic[i].y - getImageHeight(sonicbullet), sonicbullet)) {

        sonic[i].enabled = false;
        return true;
          
      }
      
    }
    
  }

  return false;
 
}

I decided to experiment with the getImageHeight() bit. I could not understand why we needed to subtract it since the XY origin is the top left of the screen. Would not subtracting the height put the collision detection above the actual position? Changing the code to the following seemed to completely eliminate the issues I having:

bool collisionSonic (int enemyX, int enemyY, const uint8_t *enemySprite) { //adapted from filmote
    
  for (uint8_t i = 0; i < maxSonics; i++) {

    if (sonic[i].enabled == true) {

      if (collide(enemyX, enemyY , enemySprite, sonic[i].x, sonic[i].y , sonicbullet)) { //- getImageHeight(enemySprite)  - getImageHeight(sonicbullet)???

        sonic[i].enabled = false;
        return true;
          
      }
      
    }
    
  }

  return false;
 
}

Changing the way the sprite is defined in the function helped eliminate the delayed and non-responsive collisions, as far as I can tell. I don’t really know why, so I will chalk it up to magic.

Removing the height modifiers altogether seemed to fix the collisions-where-there-should-be-none problem. All I could figure is that the height modifier was a way to ensure that all sprites were at ground level in the Steve the Dino tutorial. (And it turns out I was right… I guess that’s what I get for copy/pasting without thinking!)

I still think there is a bit of a timing issue with collisions though. This probably has to do with the crow sprite being drawn after checking for collisions (not confirmed). Otherwise, everything is good and I feel that I can move forward with the project.

Next up is enemy collisions with the bat.

1 Like

Well, for one thing enemySprite isn’t a pointer…

In this version it is a pointer, so that’s a step forward.

The short answer is ‘pointers’.

The long answer requires explaning what pointers are,
which requires explaining what RAM and progmem are,
and possibly what an address bus is.

If you’re up for an explanation then I’m happy to explain it,
but as I say it won’t be a short explanation.

It depends on whether its position is being modified in between the two, and if so how.


While I think of it, what even are ‘sonics’?


Does anyone happen to know where @filmote’s collision function is kept and/or whether it has a licence?

its here > https://github.com/filmote/CollideImage
BSD 3-Clause.

1 Like

I’ve made a quick attempt at improving/normalising the formatting.
I stuck to using 2 spaces becuase I expect you’re still using the Arduino IDE,
but I’ve changed the brace style to Allman style and moved the comments (most of them at least) to above what they describe rather than at the end of the line.

If you’re happy with those changes I’ll make a PR.

I can think of a number of other possible improvements,
but I don’t want to do everything at once in case you find it a bit overwhelming.

I'll leave a list of thoughts/suggestions though...
  • Make it clear that the collide function is under a BSD 3-clause licence
  • Change Collide.ino into a header file, collide.h
  • Rename Sonic (and related variables) to Bullet
  • Combine some of the sprites into sprite sheets so you can make use of the Sprites function’s frame parameter
    • E.g. Sprites::drawSelfMasked(enemyList[i].x, enemyList[i].y, crowSprite, crowFrame);
  • Create a Player struct that combines all the player variables together
  • Separate collision detection from collision resolution
  • Replace uses of if(x == true) with if(x), because the == true is redundant
  • Change the name of some functions - they should be using ‘camel case’, and should start with a verb
    • enemyspawn -> spawnEnemy
    • updateenemies -> updateEnemies
    • collisionSonic -> isBulletCollidingWith
    • enemyReset -> resetEnemy
    • drawcrow -> drawCrow
    • crow -> updateCrow
    • drawbat -> drawBat
    • batfire -> fireBullet?
  • Remove the delay logic from enemyspawn (again, see ‘separation of concerns’)
  • Use a scoped enumeration (enum class) to keep track of the bat’s glide/flap/dive state
  • Use a scoped enumeration to keep track of the enemy type
  • Clarify what modA and modB do (there might be some better options)
  • Possibly use a switch statement in drawbat
    • (I’m willing to bet I know what you were doing wrong before - most likely you left out the break; statement from each case)
  • Maybe settle on a licence for the game itself?

(Not an exhaustive list, but all I could think of at the moment.)

These look great to me.

I’ll have to look up the proper way to do that, but I agree it’s important.

This was something I was doing early on, but eventually changed everything to sonic later (bat echolocation bullets?). This was to account for the future enemy bullets. In retrospect, this is a silly concern… I could just call them enemyBullets…

This is something I wanted to do initially, but did not fully understand how the spritesheets worked. Especially because some sprites, like the bat, are not uniform in size.

This was also something I initially tried, but failed at. So, I stuck with what I knew.

I feel fairly certain that I included the breaks, but I can’t remember. Definitely needs to be revisited. Especially for the future enemy additions.

So this one is a little complicated because modA and modB do completely different things depending on the enemy type. This is a compromise to keep all the enemy variables in the array. The following are some examples.

Crow:

  • A - vertical velocity
  • B - goal Y position, or the altitude the crow tries to maintain

Skull:

  • A - shooting timer
  • B - movement timer


Bosses:

  • A - shooting timer
  • B - hit points remaining

On death, modA and modB would also be changed in different ways depending on the enemy type. I hesitate to call them anything specific for these reasons. I thought it was the simplest solution to the problem, but my knowledge is pretty limited. I made a paper list to keep track of these, but I should have transcribed that list to code comments or a readme.

I am still using the Arduino IDE. It’s convenient for me because I can run it on my laptop when I am at work. (It’s running Puppy Linux, which is not ideal)

I welcome any help or improvements as long as I can follow them (or be taught to follow them).

I thought that I had selected a license on Github, but maybe I did it wrong. That’s all new to me…

In which case, here’s a PR:

Probably best to dump the licence at the top of the file given that BSD-3 is quite a short licence.

Also, probably best to do that at roughly the same time the .ino is changed to a .h.

BatBullet would also work.

Depending on how things pan out you may be able to reuse the same structure for enemy bullets, in which case just Bullet would make sense.

Have a look at this old post, it might clear things up a bit:

That is certainly more of an issue, but it looks like that’s probably a minor problem.

Where it could be more of a problem is with the collision function.
I don’t think the collision function takes frames into account,
so it might have to be modified.

When the PR has been merged I could modify one of the things I mentioned to demonstrate.

You’d have something like:

// : uint8_t means 'use uint8_t as an underlying type'
enum class BatState : uint8_t
{
  // Starts with an underlying value of 0
  Glide,
  // The next entry has a value of 1
  Flap,
  // A value of 2... and so on
  Dive,
};

// The bat starts off by gliding
BatState batState = BatState::Glide;

// ...
// Then later you modify it like:
if (arduboy.justPressed(A_BUTTON))
{
  // ...
  batState = BatState::Flap;
  // ...
}
// ...

There are several reasons why a scoped enumeration is prefered:

  • You don’t actually have to think about what the underlying values are, you can just think of the enumeration values as special names
  • Scoped enumerations are completely type safe, you can’t accidentally use a wrong value
    • E.g. before you could do batsprite = 11; by mistake and wouldn’t know about the bug until runtime, and then you’d have a hard time tracking it down, but by having BatState batState, you cant do batState = 11, that’s an error, you can only assign one of the three designated names

I know of two solutions to this problem, but given the Arduboy’s limited resources only one of them is really viable.

Unfortunately it would take a fair bit of explaining,
because it requires both an enum class and a union to create something called a ‘tagged union’.

Basically the union allows the memory for the different enemy types to overlap, and the enumeration (the ‘tag’) identifies which enemy is currently active.

Are Notepad++ or VSCode viable options?
If so, either of those would be more suitable.

That’s why I’m trying to not do too much at once.

On a second look, you have. It’s MIT.

I was looking at the source rather than what GitHub says.
Usually people put the licence at the top of the code,
or at least at the top of the main file of the code.