search

mspraggs / potentia

Southampton Game Jam 2015
0 stars 0 forks source link

Unique SpriteKey functions for each Unit? #98

Open Fyll opened 8 years ago

Fyll commented 8 years ago

Way back when we were first putting Units in, I was puzzled as to how to convert the state of the Unit into something that could be passed to Sprite. In the end, I settled on an int that just has all of the information bit-shifted, as it was the best thing I could think of.

However, I was thinking things over recently, and realised that because the actual Unit list is in a .cpp file now, we could define functions in the .cpp for each Unit that takes the Unit as an argument, then extracts the necessary information and converts it into an int, for Sprite to convert to a Vector. The problem previously was circular dependance.

E.g. Adding in to the top of UnitList.cpp:

include "Unit.hpp"

int keyGen_Player(const Unit* unit) {
    if(unit->vel().x < 0.0f)
        //And so on...
}

Then just passing a pointer to this function into the UnitData definition, then replacing the Unit::genSpriteKey() function call with a call to this one. This would mean the Player doesn't have to risk trying to find a charging sprite, and the roller lizard doesn't have to worry about jumping or sliding down walls.

The only downsides that I can think of are that this'd bulk UnitList.cpp out quite a lot, and possibly be a pain for the Editor to parse.

What do you guys think?

DivFord commented 8 years ago

Weren't we planning to move everything out of UnitList anyway? https://github.com/mspraggs/potentia/issues/94

Could we have a general set of these functions, and just define which ones a unit has access to? So we don't have to write out the definition of "moving" for every unit, we just have to tell it if it ever needs to check for special cases like charging.

Fyll commented 8 years ago

Were we? I can see a mention of compacting things, but not removing it.

I'm sure we could always just leave Unit::genSpriteKey() to handle that in the same manner it does now, and have these functions only for special things.

DivFord commented 8 years ago

This bit in the level editor thread: "Personally I think storing it independently of the game executable is going to make things easier in the long run, particularly since we could potentially edit the text file without having to recompile the game." (Matt)

The idea is, as you say, to compact things, but that's going to be really hard to do within the syntax of C++, hence the idea that we should store the data in text files, and so be able to define our own syntax.

Fyll commented 8 years ago

Oops! I only looked at the thread you linked.

Hmm. I like the cleanliness of not having to worry about all of the Sprite cases we don't have to worry about, but I can appreciate the ease of having the data outside of the code. Unfortunately, I believe they will be mutually exclusive.

I appreciate that it's not the main concern, but recompiling a single .cpp isn't too big of a hassle (hence why I finally branched Skeleton into a .cpp).

Hmm... Perhaps a Unit creator is in order. :P

DivFord commented 8 years ago

I linked to the wrong one…

Are they mutually exclusive? Surely, if we define some high-level enemy descriptors (eg. "Charger", "Shooter"), we can use that both as a shorthand for the mechanics, and for the sprite cases. Each type of enemy would have to be hardcoded the old-fashioned (aka. "current") way, but we're likely to repeat a lot of behaviors with just slight variations. Then there's modularity to consider…

Personally, I'm less fussed by the hassle of recompiling than by the difficulty of finding my way around UnitList. Wouldn't it be nicer if this:

  unitList.push_back(UnitData(
  "Roller Lizard", //Name
  "Ul", //Code

  { Tactic({ { CON_PLAYER_IN_SIGHT, true }, { CON_PLAYER_LEVEL, true } },
             TCT_CHARGE, 0, TCX_AT_PLAYER | TCX_PUSH, TCX_FORWARDS),
    Tactic({ { CON_DEFAULT } }, TCT_GO, 0, TCX_RIGHT | TCX_SPOT_PIT, TCX_FORWARDS)
  }, //Tactics

  getConstant<float>("PLAYER_ACCELERATION") * 2.0f, //Acceleration
  getConstant<float>("PLAYER_MAX_SPEED") / 4.0f, //Max speed
  0.05f, //Toughness

  PATH_ROLLER_LIZARD_SPRITES, //Sprite sheet
  "", //Available weapons.

  getConstant<Vector>("TILE_DIM") * Vector(56.0f / 64.0f, 25.0f / 64.0f), //Unit dimensions
  getConstant<Vector>("TILE_DIM") * Vector(4.0f / 64.0f, 18.0f / 64.0f), //"Unit -> Sprite" offset
  getConstant<Vector>("TILE_DIM"), //Sprite dimensions

  { { { { 0b000000 }, { Vector(0.0f, 0.0f), 1 } }, { { 0b000001 }, { Vector(0.0f, 0.5f), 1 } }, //Stand
      { { 0b000010 }, { Vector(0.0f, 0.125f), 8 } }, { { 0b000011 }, { Vector(0.0f, 0.625f), 8 } }, //Walk
      { { 0b001000, 0b001010 }, { Vector(0.0f, 0.25f), 1 } }, //Jump right
      { { 0b001001, 0b001011 }, { Vector(0.0f, 0.75f), 1 } }, //Jump left
      { { 0b010000, 0b010010 }, { Vector(0.125f, 0.25f), 1 } }, //Fall right
      { { 0b010001, 0b010011 }, { Vector(0.125f, 0.75f), 1 } }, //Fall left
      { { 0b100000, 0b100010, 0b100110, 0b101000, 0b101010, 0b101110, 0b110000, 0b110010, 0b110110 },
        { Vector(0.0f, 0.375f), 1 } }, //Die right
      { { 0b100001, 0b100011, 0b100111, 0b101001, 0b101011, 0b101111, 0b110001, 0b110011, 0b110111 },
        { Vector(0.0f, 0.875f), 1 } }, //Die left
      { { 0b000110, 0b001110, 0b010110 }, { Vector(0.25f, 0.375f), 5 } }, //Roll Right
      { { 0b000111, 0b001111, 0b010111 }, { Vector(0.25f, 0.875f), 5 } } //Roll Left
    } }, //Sprite map
  { { { { { { Vector(0.0f, 0.0f), 1 }, { Vector(0.0f, 0.5f), 1 } },
          { { Vector(0.0f, 0.0f), 1 }, { Vector(0.0f, 0.625f), 8 } },
          { { Vector(0.0f, 0.125f), 8 }, { Vector(0.0f, 0.5f), 1 } },
          { { Vector(0.0f, 0.125f), 8 }, { Vector(0.0f, 0.625f), 8 } } },
        { Vector(0.25f, 0.0f), 5 } }, //Turning right to left
      { { { { Vector(0.0f, 0.5f), 1 }, { Vector(0.0f, 0.0f), 1 } },
          { { Vector(0.0f, 0.5f), 1 }, { Vector(0.0f, 0.125f), 8 } },
          { { Vector(0.0f, 0.625f), 8 }, { Vector(0.0f, 0.0f), 1 } },
          { { Vector(0.0f, 0.625f), 8 }, { Vector(0.0f, 0.125f), 8 } } },
        { Vector(0.25f, 0.5f), 5 } }, //Turning left to right
      { { { { Vector(0.125f, 0.25f), 1 }, { Vector(0.0f, 0.0f), 1 } },
          { { Vector(0.125f, 0.25f), 1 }, { Vector(0.0f, 0.125f), 1 } } },
        { Vector(0.25f, 0.25f), 1 } }, //Landing (right)
      { { { { Vector(0.125f, 0.75f), 1 }, { Vector(0.0f, 0.5f), 1 } },
          { { Vector(0.125f, 0.75f), 1 }, { Vector(0.0f, 0.625f), 1 } } },
        { Vector(0.25f, 0.75f), 1 } } //Landing (left)
    } }, //Animation map

  1.0f, //Mass (in terms of player masses)
  DIR_DOWN, //Direction of gravity
  true, //Lethal
  false, //Can jump

  true, //Solid
  0.0f, //Coefficient of drag/restitution

  EFF_NONE)); //Effects

Were just:

Roller Lizard
Ul
Charger, Pacer
Accel: 4.0
Max Spd: 0.25
Toughness: 0.05
PATH_ROLLER_LIZARD_SPRITES
SpriteDims (64.0, 64.0)
SpriteOffset: (4.0, 18.0)
ColliderDims: (56.0, 25.0)
EFF_NONE
Fyll commented 8 years ago

My worry is that all of that code is going to have to be written somewhere, and all we're doing is displacing it. While many behaviours and info and the like would be shared, the big and bulky bits are big and bulky specifically because they are essentially a collection of hard-coded points about this thing.

Take for example the hitbox/animations. While some parts of that will be common, if we end up going for hitboxes for specific animations and sprite sets then it's all suddenly unique from every other Unit.

Sections of the hitbox data can be reused between Units, but incorporating that could be pretty awkward, as compared to a simple list.

DivFord commented 8 years ago

Crap. I forgot about sprite-specific hitboxes. Still, if we ever do palette-swapped or re-skinned enemies (which I assume we will…), it would be nice to not have to write out the hitboxes for each sprite all over again.

I suppose what I want is to break the unit definitions into component pieces, then define a unit as a list of components.

Yes, there would have to be a fairly hefty interface to accommodate this, but in the long run, I think we'll spend a lot more time making content than editing the engine. I hope so, anyway.

Fyll commented 8 years ago

That's what ctrl+c and ctrl+v are for. :P

Some things could probably be compacted down into components, but the bulkiest bits are bulkiest for a reason.

I was only joking when I suggested the Unit creator. There's no harm in creating one, but I don't think we'll be adding Units fast enough to justify it.

Fyll commented 8 years ago

I've... kinda relented on my resistance to splitting UnitList.cpp up. More specifically, I managed to persuade myself this morning that it would be a good idea to split each of the Units off into their own .cpp (probably tucked away in a folder). I haven't actually done this, mind, just persuaded myself that it'd be a reasonable thing to do.

Ignoring hitboxes and sprites, I'd be fine with what you're suggesting, so what if we did a nice readable format for the bits that can be done that way (name, speed, jobs, etc.), but just stuck the rest at the bottom of the file in code. It'd be a mess, but the mess will have to exist somewhere.

Arsed'ness be willing, we could possibly just print hitboxes onto the spritsheet in magenta, then read it off from that, but I don't know if I like the idea of trying to interpret hitboxes pixel by pixel....

DivFord commented 8 years ago

I like your take on it. As you say, the mess has to exist, but I'm fine with that so long as the cleaner data isn't mixed into that mess.

Reading the hitbox from a texture actually wouldn't be that tricky. Assuming it's still a box (which seems sensible), we just have to find the top-left and bottom-right magenta pixel. That surely isn't that hard to do. The ideal is probably some sort of utility script which we can run from command line to write the values into the data files, rather than trying to do it at runtime.

What's your opinion on sub-classing units? i.e. "red lizard" would inherit from "lizard", and so avoid re-stating the hitboxes etc.

Fyll commented 8 years ago

Identifying it in that way wouldn't be so bad (I was trying to think about funnily shaped hitboxes, but we can probably ignore them). I'm not too sure how to remove the magenta pixel from the image when loading it into the game though (at least, not in a nice, clean, fast way). Unless we have a separate set of images for identifying hitboxes....

I'd be okay with that. It does bring up a question though. If the hitbox is just lumped at the end of the file, would the child unit just be inside the same file as his parent, or would he have the parent's name at the bottom instead of a hitbox? I prefer the sound of the former (although I haven't thought too much about it), so it would be something like:

Name: Roller Lizard
Code: Ul
Tactics: Charger, Pacer
Accel: 4.0
Max Spd: 0.25
Toughness: 0.05
Sprite: PATH_ROLLER_LIZARD_SPRITES
Palette: WHITE
SpriteDims: (64.0, 64.0)
SpriteOffset: (4.0, 18.0)
Effects: EFF_NONE

Child:
Name: Red Lizard
Code: Ur
Palette: RED

hitbox stuff

There's actually code already in place for putting a monochrome filter over an image, it's just that nothing uses it. This may not actually be what you were thinking, but it'd be a cheaty way to reddify a lizard.

Looking at that has made me wonder, is this a .txt or a .cpp? If it's a txt, the constants and enums won't be defined (which'll be a pain), but if it's a .cpp it'll need some C++ formatting (which'll make it a lot uglier).

DivFord commented 8 years ago

A second image is probably the best solution. We have plenty of space on the sprite sheet, we'd just need to add anim-maps for the collision textures.

Actually, I was thinking that if we had each unit be a C++ class, we could use the normal inheritance system to get the hitbox/sprite data. Thinking about it, I maybe didn't quite remember how classes work… I suppose it would only work if each data entry was a function. Something like:

string UnitName () { return "Roller Lizard"; }
string Code () { return "Ul"; }
tactic[] Tactics () { return [CHARGER, PACER]; }

It's been a while since I wrote anything C-related, so I may be mixing stuff up a bit.

I'll run some tests, but I suspect a simple color filter wouldn't look very good. Nice to know the option is there though.

Obviously my suggestion requires it be a cpp, which may be an argument against it. I don't think it has to make it unreadable though, so long as we make good use of line-breaks and whitespace.

Fyll commented 8 years ago

Using C++'s inheritance implies that each of the Units is its own class, which I don't think would necessarily be the best idea. You're right in that we'd have to do everything returning as functions, so it would be something like:

class RollerLizard
{
    public:
        virtual const std::string& name() { return "Roller Lizard"; }
        virtual const std::string& code() { return "Ul"; }

        etc.
};

class RedLizard : public RollerLizard
{
    public:
        virtual const std::string& name() { return "Red Lizard"; }
        virtual const std::string& code() { return "Ur"; }

        virtual const std::string& filePath() { return PATH_RED_LIZARD_SPRITES; }
};

hitbox stuff

I was just thinking to treat them as completely separate Units, just happening to read the same few lines to get the hitbox data. In C++ terms, I could get this down to (I think the tactics constructor would be a bit more complicated, but eh):

const UnitDataStuff UnitRollerLizard({
    { "Name", "Roller Lizard" },
    { "Code", "Ul" },
    { "Tactics", { TCT_CHARGE, TCT_PACE } },
    { "Acceleration", 4.0f },
    { "Max Speed", 0.25f },
    { "Toughness", 0.05f },
    { "File Path", PATH_ROLLER_LIZARD_SPRITES },
    { "Sprite Dims", Vector(64.0f, 64.0f) },
    { "Sprite Offset", Vector(4.0f, 18.0f) },
    { "Effects", EFF_NONE },
});

class UnitDataStuff UnitRedLizard(UnitRollerLizard, {
    { "Name", "Red Lizard" },
    { "Code", "Ur" },
    { "File Path", PATH_RED_LIZARD_SPRITES },
});

hitbox stuff
DivFord commented 8 years ago

That's actually pretty accessible, but according to Issue 94, it's missing: Guns Mass Gravity Direction (not convinced this is needed…) Can Jump Solid Coefficient of friction Metadata Tags

Fyll commented 8 years ago

Well, aside from the metadata tags ('cause I have no idea what that'll look like), the rest could be added with no problem (I'd write this such that the second parameter can be anything, so I imagine that anything could be described this way (you are talking about the second one, right?)).

DivFord commented 8 years ago

Yes, I meant the second one. Metadata tags is just a list of strings, so shouldn't be a problem.

Would we be taking the same sort of approach with the other object types? Consistency would be nice, and this does look like a format which would be suited to reading out to the editor as mentioned in the other thread (i.e. the thing that would let the level editor and game read from the same files).

Having said that, the unit inheritance stuff (eg. "red lizard") might make editor access more complicated.

Fyll commented 8 years ago

Maybe in the long run, but we should probably only worry about one thing at a time. :P Specifically, I got persuaded for Units because UnitList.cpp was starting to take a non-trivial amount of time to open.... I would definitely think that it'd be good to do, but a good thing to do after getting it all working with Units.

We don't necessarily have to bother with that. The bulkiest bit (the hitbox stuff) is tucked away outside, so it shouldn't matter too much if we repeat some basic stuff. Not that I know how the editor works, but would it help if, instead of passing UnitRollerLizard as a separate argument, I stuck a { "Parent", UnitRollerLizard }, at the front of the red lizard's list of things? That's how the code would be handling it (i.e. if this argument is passed, fill everything with that's data, then overwrite what the later list elements tell you to overwrite).

DivFord commented 8 years ago

I went and re-read the Level Editor thread (Issue 93). It looks like we basically made up our minds in that one that txt files would be better than cpp, since it would save us compiling whenever we tweaked data values. On the other hand, if every unit had its own cpp, the compile time for changing a single unit would presumably be very low. Thoughts?

The editor interface basically wants to use the command line to grab data for the unit. It actually only cares about five of the variables (and one of them is "allowed zones", which doesn't currently exist. Hmm…).

EDIT: Sudden thought; what happens if you put a "main" function in the unit data cpp file? Could I call that to get the data I need? Would it interfere with the actual game?

Fyll commented 8 years ago

I would rather use .cpp's, even if only for the ability to use enums and constants without having to write a thing to connect their names with their values (i.e. a great big switch/if).

Maybe we could just make sure to define those 5 variables for each Unit, inherited or not? Or if the things it cares about are all inherited, you could just have it read them from the parent (or some combination)?

DivFord commented 8 years ago

That's reasonable. The five things are:

None of them are constants or enums, so that's nice. I think we could happily re-define these for every unit variation. Also, would you be happy for these to be the first five things in the unit definition? If so, that would make it easier to parse the file when the editor searches for them.

PS. I found out what happens if you put a second "main" in another file… My computer is still recovering.

Fyll commented 8 years ago

I wouldn't have any problems with that, as long as name and code were the first two.

Are you having a go at this then?

DivFord commented 8 years ago

Matt seems to be busy, so if you're going to replace UnitList with this new system, I'll see about the editor integration. Mainly because it looks a lot simpler now than it did originally :P I'll let you do your part first though.

EDIT: Though to be fair, what's really stopping us from using the editor at the moment is the fact that the game can't use the files it outputs. Not sure I want to tackle that one...

Fyll commented 8 years ago

Wasn't there a python script to compile the levels into levels.cpp?

DivFord commented 8 years ago

There is, but that was a different problem. The problem was that the game still read the old level format we created for the jam, not the one the editor outputs. However, I had a crack at it, and it turns out not to have been that difficult to change. Even an artist can do it!

I now have it loading most of the data from the new files. No metadata yet, and I'm still working my way through the patterns, but so far it's going well. Also, it seems the editor is using the wrong codes for units, but I'll fix that later.

Fyll commented 8 years ago

As for this (I still haven't started yet :( ), I just realised that having it all as .cpp's means that if we edit Constants.hpp at any point, it will mean a lot of recompiling....

DivFord commented 8 years ago

"I would rather use .cpp's, even if only for the ability to use enums and constants without having to write a thing to connect their names with their values (i.e. a great big switch/if)." <- Given that this would avoid the compiling problem, maybe the txt files are worth the effort...

Fyll commented 8 years ago

I suppose....

I'll have a go at writing some text to UnitData translator at some point then, as it's probably not quite as bad as I'm making it out to be. Basically, it comes down to the question of: Do we want to have to edit a giant multikey map (hey, that one in Sprite might actually be useful somewhere else!) every time we add or remove a constant or enum, or do we want to do a giant recompile every time we edit Constants.hpp?

DivFord commented 8 years ago

I vote for the multikey map. The ones for sprites are only a pain to edit because they're mapping bitmasks to vectors, neither of which is very easy to read. A map for constants would presumably be strings on one side, numbers on the other.

If we can cope with:

{ { { { 0b000000, 0b000100 }, { Vector(0.0f, 0.0f), 1 } }, //Stand left
    { { 0b000001, 0b000101 }, { Vector(0.0f, 0.5f), 1 } }, //Stand right
    { { 0b000010, 0b000110 }, { Vector(0.0f, 0.125f), 8 } }, //Walk left

Surely we can manage:

{ { { {"EFF_NONE"}, { 0 } },
    { { "EFF_AUTO" }, { 1 } },
    { { "EFF_DEATH"  }, { 2 } },

Of course, I don't fully understand this part, so my wild guess as to what it might look like could be completely off...

Fyll commented 8 years ago

That's pretty much how I see it being (except probably with less brackets).

Fyll commented 8 years ago

I've had a first attempt at this and there are two problems I've stumbled upon:

DivFord commented 8 years ago

I take it you've gone for the txt option?

Personally, I would go for the file full of names option. It seems simpler, and thus less likely to break. If there's a downside, I can't see it.

Could you maybe use the first character of the value to distinguish what it is? Sort of like Hungarian notation. So "C_SOMETHING" is a constant called "SOMETHING" and "S_SOMETHING" is a string "SOMETHING". You'd just have to discard the first two characters. Not sure if it's necessary, but it seems safer than relying on an implicit naming convention.

Fyll commented 8 years ago

Yeah. I figured I'd at least see how it looks.

Fair enough, the main downside I was thinking about was exactly that we'd then have to write all of the filenames in a file (not much of a downside I admit, but it'd depend how lazy we were feeling).

That'd be lovely, but I thought the point of this was that the .txt files were meant to be human-readable and sticking extra characters in may make that a bit more awkward. Would we ever use all caps for anything other than constants or enums?

DivFord commented 8 years ago

Lazy enough that I'd rather write a name in a header file than work out another library?

See, that's why I added the underscore. Makes it nice and easy to skip over that first character. I want to say that we won't ever use all caps for other stuff, but it seems like a very easy thing to forget about. What happens when we add a property to units for printing something to the screen (boss names? dialogue? I don't know, roll with it) and we want to have something shouty? The point being that there is a chance, however small, that this may cause problems in the future, and since it doesn't cost much to do it "properly" now, I think we might as well.

Fyll commented 8 years ago

Fair enough. I'll use C_ for constants, E_ for enums, and leave strings as they are (as I think it's safe to say that we won't ever start anything with either of these, and strings should probably be the most readable (and may have spaces which would look wierd with an underscore jammed at the front)).

Fyll commented 8 years ago

Okay, I've pushed an attempt at this. Floating platforms don't have any tactics, but I wanted to see if anyone had something to say about it so far.

DivFord commented 8 years ago

I get this error when trying to compile the new code:

In file included from /Users/Div/Documents/Games/Global_Game_Jam_2015/potentia/lib/UnitList.hpp:6: /Users/Div/Documents/Games/Global_Game_Jam_2015/potentia/lib/Sprite.hpp:101:38: error: implicit instantiation of undefined template 'std::1::hash<std::1::vector<int, std::__1::allocator > >' auto vector_hasher = hash< vector >(); ^ /usr/bin/../lib/c++/v1/memory:3076:29: note: template is declared here template struct hash; ^ 1 error generated.

I assume this is down to compiler differences, but I don't really understand templates.

Fyll commented 8 years ago

Oh dear. I kind of just poked that code until it worked for me.

Try adding #include<functional>, which might help. If it doesn't, maybe try #include<tr1/functional> instead.

DivFord commented 8 years ago

Nope. Second one not recognized, first one does nothing.

Fyll commented 8 years ago

I've pushed a possible fix. It seemed to still work for me.

mspraggs commented 8 years ago

From the error message it looks like clang already has a hash function for std::vector types. The fix would be some preprocessor flag to determine whether clang is being used, then act accordingly.

Also, it would be well worth testing the hash function implementations to make sure they're collision free, as if they're not this is the kind of thing that could come back to bite you later. This could be simply generating a random pair of the objects to be hashed and then checking whether their hashes are equal, then repeating this process a thousand times, say.