Generalize/Categorize items

[cdsupina]

Affects #129

While I was working on writing the section for adding an item to the game (I'm using a sniper rifle as an example), I noticed that a lot of the code I was writing for the sniper rifle was directly copying the musket code. I think that we could reduce code and lower the barrier of entry to contributing to the game by generalizing items into categories. For example, instead of having src/items/muscet.rs we should have src/items/basic_gun.rs. Then we can create a general gun struct:

pub struct Gun {
    pub gun_sprite: GunlikeAnimation,
    pub gun_fx_sprite: GunlikeAnimation,
    pub gun_fx: bool,

    pub bullets: i32,

    pub body: PhysicsBody,
    pub throwable: ThrowableItem,
    pub guntype: GunType,
}

Then we can implement contractors for all the different types of guns.

impl Gun {
    ...
    pub fn spawn_musket(pos: Vec2) -> HandleUntyped {
        ...
    }

    pub fn spawn_sniper(pos: Vec2) -> HandleUntyped {
        ...
    }
    
    pub fn shoot(&self, node: Handle<Muscet>, player: Handle<Player>) -> Coroutine {
        match self.gun_type {
            Sniper => {
                ...
            },
            Musket => {
                ...
            },
        }
    }

    ...
}

We should also generalize bullets in a similar way.

Overall this will lower the barrier of entry to contributing to the game, because new coders/contributors could start off just modifying the gun struct for their new gun with adjusted values.

[erlend-sh]

Sounds good! The only consideration on my mind is the extent to which we wanna introduce this right away. Could you do this for just the gun(s) at first without touching any of the other items?

I agree this is lowering the barrier to entry once it’s the new standard everywhere. On the other hand we don’t want to be too strict about fitting everything into a neat category when a new weapon is initially being implemented, as I’ve pointed out on chat:

Don’t worry too much about modularization in the first pass. Easier to sort through that in a second pass once more items are in.

I feel like there’s two essential steps to making a new item:

1. Do whatever works: duplicate code and hack it up to your heart’s content. The novelty of the item behavior is the priority.
2. Clean it up. Reduce duplication and hacky workarounds. Reducing tech debt is the priority.

This could could all happen in one PR starting in draft form, or it could be two separate ones. Of course, some items are also simple enough that they won’t need to go through a Whatever Works stage.

[legendiguess]

Yeah, I also was thinking of items categories when i worked on shoes item reimplementation (both turtleshell and shoes can be categorised as "powerup" item, i believe). But i rather prefer still make one file .rs per weapon with one abstraction file, than your solution(where abstraction+items all in one file, correct me if i got it wrong). More understandable for me, and, if we make new abstraction gun like you suggested, all files that use it will be very small, instead of one big gun.rs with all weapons. Also think of case, where two contributors will be working on gun.rs to add new guns, instead of each working in his file, it can be harder to merge.

[cdsupina]

Sounds good! The only consideration on my mind is the extent to which we wanna introduce this right away. Could you do this for just the gun(s) at first without touching any of the other items?

I can do this for just the guns, that way we can see what people think when I make the PR.

My thinking was that if we want to divide contributions into difficulty levels, a level 1 item contribution would be adding a new type of gun where you can basically just create new constructor for the existing gun struct using different values. A level 2 item contribution would be a "whatever works" item where you are trying to get a new unique kind of item in the game.

But i rather prefer still make one file .rs per weapon with one abstraction file, than your solution(where abstraction+items all in one file, correct me if i got it wrong).

I was thinking of putting everything in the gun.rs file, but I think you are right, it makes more sense to still create a sniper.rs file and just create another impl Gun block in that file.

[orhun]

From the abstraction standpoint, I think it would be more appropriate to have traits that define the required functions to implement a similar struct. For example,

pub trait Shooter {
	fn shoot(...);
    fn shoot_predefined(...) {
		...
    }
}

pub struct Gun { ... }

impl Shooter for Gun {
	fn shoot(...) {
		...
	}
}

pub struct SniperRifle;

impl Shooter for SniperRifle { ... }

However, I'm not sure if this approach is fully applicable for the FishFight codebase. It might require some refactoring I think.

[grufkork]

What I think would be neat is if each gun has its own file, so there isn't one megafile of all guns. All guns share the same behaviour except how/what they fire, sprite and ammo count. So that's all that needs to be implemented for each gun, and the rest of the behaviour is copied (like virtual classes in C#). I suppose the way to do this is a non-polymorphic GunItem. The GunItem contains all the pickup logic, physics, etc. The GunIten also has a reference to a Gun/Shootable/whatever trait. That trait defines a fire() function and supplies a struct with some info regarding the gun (ammo cap, sprite). That way each gun file should be pretty small, with minimal amounts of boilerplate code required. I guess this is what orhun is on about?

[cdsupina]

I'll try the trait approach and we can review it again when I make the PR.

[olefasting]

IMO, it is better to make a base data struct (Weapon struct, for example) and separate the logic that varies from implementation to implementation, as @orhun mentioned. My preference is to just use closures or coroutines for this, in stead of traits, as they have way less overhead. No matter how we solve it, I would be careful making "super-classes" that hold all data and logic for a category as big as weapons, for all the obvious reasons, but also because we have a lot of contributers on this project and it will greatly complicate conflict resolution, when many people work on different weapon implementations, etc

[cdsupina]

#136 Furthers this discussion a little bit. We settled on the no trait method for generalizing, but there is definitely room to add traits in the future. For example, certain weapons may share some kind of firing behavior. This could be shared between Gun and other weapons using a potential Shootable trait. I'll leave this issue open for further discussion, as I feel there still needs to be more thought put into this.

[olefasting]

In stead of using traits, like that, I think it is better to create one Weapon trait with an attack method, or something similar. If we later create an Item supertrait, with pickup and drop behavior, we just make it a prerequisite for Weapon. Working with dynamic trait objects can be a bit limiting, but it shouldn't really apply in this case, as it is mostly applicable for trait methods with dynamic types in receiver or as return types .

[cdsupina]

I recently created the items page of the documentation. While I was creating this section and testing out all of the items I found that items could generally be categorized like the following:

• Guns: Musket, Sniper, Machine Gun, Cannon
• Melee: Sword
• Wearables: Turtle Shell, Shoes
• Environmental: Galleon, Volcano, Shark Rain
• Explosives/Throwables: Grenades, Mines, Jellyfish
• Arena: Sproinger

I thought I'd leave a note of this here for anyone that wants to address this further.

[zicklag]

With the Bevy rewrite the whole organization changes and this ends up somewhat out of date. This is something that we'll be working out as we go also while we figure out how scripting and script performance will play out, which has a big effect on how we organize items.

We can open more specific issues as we find out more of what we'll be doing.