search

mattconsto / lost-the-game

Deserted. Created in 48 hours as part of the Global Game Jam 2015.
https://mattconsto.itch.io/lost-the-game
3 stars 0 forks source link

Refactor #38

Open mattconsto opened 9 years ago

mattconsto commented 9 years ago

We are no longer time limited, one of the things we should be doing is gradually be refactoring the code base so it's no longer full of dumb decisions. This means encapsulation and a sane architecture.

mikejewell commented 9 years ago

Definitely for a release version, but maybe not too essential for a prototype we can share/publicise. Doesn't mean we can't refactor as we go, of course :)

mattconsto commented 9 years ago

Organised the files, and as a bonus, it will now run as a single fat jar. So some progress has been made. No idea how to assign a patch to a comment.

agjlewis commented 9 years ago

Sounds like a good plan, would be nice to get it a bit more sane, time pressure def leads to compromises !

mikejewell commented 9 years ago

As part of this, I'm distilling ActionManager a little: createConsumeAction and createCraftAction produce IActionable objects that can be used to build an Action. So, if you want to get some food points when you eat a fish:

new Action("Eat Fish", "eating a fish", 10, createConsumeAction(ItemType.FISH, 0, 0, 30));

This checks that you have a fish, removes it from your inventory, and gives you 30 food points after it's eaten.

You can also pass in an IActionable to provide 'hooks' in case you want to provide extra functionality. If you want to craft a brick from mud and grass, and make a squelch noise, you can use:

new Action("Make Mud Brick",
"making a mud brick", 10, createCraftAction(
      new ItemType[] { ItemType.MUD, ItemType.GRASS },
      new int[] { 1, 1 },
      ItemType.BRICK, new BaseActionable() {
        @Override
        public void afterAction(GameSession gs, Agent agent, TileSystem ts, Tile tile, 
          MonsterManager monsterManager) {
        SoundManager.playSound("squelch");
       }
}))

This consumes 1 mud and 1 grass, and creates a brick. The BaseActionable lets you skip implementing every method - so I just override afterAction and play the sound effect.

Does this look sensible? Thinking we can do similar for the foraging, dirt digging, etc. At least reduces the lines of code in that mega-file :)

mattconsto commented 9 years ago

I'm not fond of that crafting code tbh, it reeks of code smell. Seems too complex for what should be fairly simple. Honestly I have barely touched the code that handles the inventory but I think it would be nicer if we could just call a function like this. This code assumes craft returns a boolean stating if it was crafted, although we could have it return the number crafted or whatever.

if(inventory.craft(agent, ItemType.BRICK, 1))
    SoundManager.playSound("squelch");

If I was implementing it I would look into extending ItemType so it contains the type's name, description, recipe (Although that may be awkward to do without resorting to just using a String instead of an enum value due to circular referencing(?)) and anything else.

As for the consumable, something like this would just abstract away the details. Do we really care if eating a fish restores 30 health in ActionManager?

if(inventory.consume(agent, ItemType.FISH, 1))
    SoundManager.playSound("nom");

Bonus of doing this, means that if we add more food or more crafting recipes we don't need to make many changes at all. The simpler the interface, the better.

mikejewell commented 9 years ago

I'm totally up for good ways to simplify it! The create* methods are partly me trying to get it down to the various cases that we need (i.e. consume, craft). I'd actually be more tempted to put consume/craft into Agent - as that's what's doing the consuming/crafting. So something like:

agent.craft(ItemType.BRICK, 1) agent.consume(ItemType.FISH, 1)

Agent will (eventually) have its own Inventory anyway, and it should be able to find out where it's standing (the tile is important for some actions). Plus it can access its own health, etc.

It would be good to handle side-effects on a per-ItemType basis (e.g. there's a chance you might eat a poisonous fish or berries) - but it'd be nice not to have to add too many new classes.

Actions also split nicely into inventory-based and tile-based - I envisage the inventory ones being similar to the buttons we have currently, while tile ones could potentially be in some sort of radial menu around the player (so things like 'pick berries'). So maybe we should take ItemType and TileType and extend those...

The IActionable still makes sense - as we need a way to split up the tasks (i.e. if you walk away mid action; if we have actions that take resources over time; if we want more action queueing in future), but it could certainly call methods on the Agent/Tile itself.

mikejewell commented 9 years ago

So! Some initial work in progress has gone in:

For example:

    addAction(new EatAction("Eat Fish", "eating a fish", 10, ItemType.FISH));

Handles eating a fish (button name, button description, how long it takes, and what you're eating)

addAction(new HarvestAction("Pick Grass", "picking some grass", 6,
                ItemType.GRASS, TileId.GRASS, TileId.DIRT, 1));

Handles harvesting grass: it takes 6 'minutes', gives you ItemType.GRASS, and transforms a TileId.GRASS to a TileId.DIRT after 1 harvest.

addAction(new CraftAction("Make Axe", "making an axe", 15,
                ItemType.AXE, new ItemType[] { ItemType.STICK, ItemType.METAL,
                        ItemType.VINE }));

Crafts an axe in 15 minutes, from a stick, metal, and vine. You can also pass in an array of ItemStack - this is just constructed with new ItemStack(ItemType,STICK, 10) or similar, so you don't have to to have 10 separate ItemType.STICKs.

I think this is about as simple as we can get it for now: the IAction interface allows for the splitting up of actions (and, eventually, queuing), so is required here. You'll see that EatAction, CraftAction, HarvestAction, OtherAction are pretty simple classes - they just farm things off to the inventory and agent. In fact, the callbacks (onStart, onComplete, canPerform) are just passed the Agent that is performing the action, rather than all the extra params we had before.

The idea of a RecipeBook containing all the actions is very similar to that in Minecraft - makes it really simple to add new recipes, and keeps it all in one place.

In the process, I've also added GameConfig. This contains most of the constants for the game - how quickly you get hungry, how many game minutes you have per second, etc. Again, it's nice to have the gameplay easily tweakable in one or two files. GameSession is a little simpler too - the parts to do with inventory generation and agent updating are now in the Agent itself, which makes more sense I think.

steppers commented 9 years ago

Great work, I wasn't sure if people were still working on it! That system seems brilliant. I've been experimenting with different ways to implement the new rendering over the last few days.

jemeson007 commented 9 years ago

Great guys, Wanna add a new member to the team. What do you guys reckon? On 5 Feb 2015 23:39, "Oliver Steptoe" notifications@github.com wrote:

Great work, I wasn't sure if people were still working on it! That system seems brilliant. I've been experimenting with different ways to implement the new rendering over the last few days.

— Reply to this email directly or view it on GitHub https://github.com/mattconsto/gamejam/issues/38#issuecomment-73153882.

mikejewell commented 9 years ago

Hey! Given the code's having a fair bit of a reshuffle at the moment, it might be better if he starts off making a fork of the repository, and makes pull requests - that way we can steadily bring changes in without so much coordination, and it also marks a nice step on getting it into the community some more. See: https://help.github.com/articles/using-pull-requests/