Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add effects for permanent stat increase #75

Open
Grimmys opened this issue Aug 17, 2023 · 6 comments
Open

Add effects for permanent stat increase #75

Grimmys opened this issue Aug 17, 2023 · 6 comments
Labels
enhancement New feature or request good first issue Good for newcomers text Relates to text visible in the application

Comments

@Grimmys
Copy link
Owner

Grimmys commented Aug 17, 2023

Currently, only effects giving temporary stat increase as an alteration are present (such as speed_up, strength_up or defense_up).
Nothing for increasing the stats in a more "definitive" way.

It would be nice to add these kind of effects, and why not a few items making use of them.
These effects should not result in the creation of an Alteration (since it would not be temporary), but should rather directly modify the stats of the targeted entities.

Place to add implementation of the effects is effect.py.
Place for defining displayed names is in text.py (effects sub-dictionary in TRANSLATIONS).

No need to bother about translations to other languages than English right now, checking if the game is not crashing when using another language should be sufficient.

@Grimmys Grimmys added enhancement New feature or request good first issue Good for newcomers text Relates to text visible in the application labels Aug 17, 2023
@IliyasDev
Copy link

Maybe instead of having to edit the effects in effect.py, you should think of making an array on the character.py that stores a tuple of the effect ID and the duration of the effect and have the effect.py just be a handler that takes that array and updates the character stats.

@Grimmys
Copy link
Owner Author

Grimmys commented Aug 18, 2023

I'm not sure to get your point.
First, effects don't have any duration, it's alterations have one.

The idea behind effect/alteration distinction is that an effect is any kind of change an entity may be affected by, because they drank a potion, an item was thrown at them, a specific spell hit them etc., and an alteration is more like a change in status, that could be temporary with a hard turn limit (duration), or we could imagine it being more resilient and requiring to consume something (like an antidote) / go somewhere (like to a "medical center") to get rid of it.
An alteration is likely being caused by an effect.

An example from the code for the stun effect:

# At instanciation of the effect
if self.name == "stun":
    alteration_root = etree.parse("data/alterations.xml").find(name)
    desc = get_localized_string(alteration_root.find("info")).strip()
    abbr = alteration_root.find("abbreviated_name").text.strip()
    effs_el = alteration_root.find("effects")
    durable_effects = (effs_el.text.strip().split(",") if effs_el is not None else [])
    self.alteration = Alteration(self.name, abbr, self.power, self.duration, desc, durable_effects)
# When it's being applied to an entity
if self.name == "stun":
    entity.set_alteration(self.alteration)
    msg = f_ENTITY_HAS_BEEN_STUNNED_FOR_NUMBER_TURNS(entity, self.duration)

Stun is a kind of effect that apply the Stun alteration for X turns to an entity.

And I'm not sure if anything like the list of effects should be stored in Character entity... First because not only Characters could be affected, but also Foes, or other kind of entities. And also because I don't know, they don't look so closely related in my opinion. Effects should be kept rather close to the Effect class, so everything related to effect is around the same place.

But maybe I just misunderstood your idea.

@IliyasDev
Copy link

So my point was having a hashmap containing effects and there duration and if an effect has a duration that is not 0 the effect will be applied, but if the duration is none the effect will stay forever, and then you have a the effect take the entity hit by the potion or the attack with effect, and trigger a applyEffectEvent in the entity code, then the entity's code detect that there is a change in the duration of an effect, and applies it to the stats.
I don't know much about the effect in your game so I'm going to make my own for an example:

class Entity:
    # Just a draft of the initialization process from a effects only POV
    def __init__(self):
        self.effects = {'stun': 0, 'speed': 0, 'health_boost': 0, 'strength': 0}
        self.entityType = 'player'
        self.speed = 60
        self.strength = 50
        self.health = 100

    # Very rough, but you get the point
    def applyEffectEvent(self, effect, duration):
        if not effect in self.effects:
            return f"Effect doesn't apply on {self.entityType}"
        self.effects[effect] = duration
        self.updateState()
        return "Effect applied"
    

    # I wrote my updateState function to just handle effects
    # Normally it handles everything that needs to be updated
    def updateState(self):
        for effect in self.effects:
            if self.effects[effect] == 0:
                return
            
            if self.effects[effect] == None:
                match effect:
                    case "speed":
                        self.speed += 20
                    case "strength":
                        self.strength += 10
                    case "health_boost":
                        self.health += 50
                    case "stun":
                        self.speed -= 40
                return
            
            # Here you put how you usually apply your effects with a duration
            # Just use self.effects[effect] to get the duration in whatever unit

@Grimmys
Copy link
Owner Author

Grimmys commented Aug 19, 2023

Ok to be really honest I'm still not sure to get you... It's just that I need to dig further on the subject maybe.

Having an hashmap (dict in Python) is an idea I like: maps/dict is always better than a sequence of if/else statements in my opinion, it's cleaner.

But I don't understand why effect logic should be in entity instead of in... Its own class. And I don't get what would be the pros of following this approach.

@IliyasDev
Copy link

IliyasDev commented Aug 19, 2023

OK I'll explain further:

Using a Hash Map to store the effects on the entity instead of on a separate class :

Pros :

  1. Assume you have multiple entity types, humanoids, goblins, elfs, animals... Having each class for these entities have its own Hash Map of effects is beneficial, because there might be some effects that should not work on that type, example:
  • Blindness on a Goblin shouldn't work because they might already live in the dark, they can't see either way
  • Strength on an Elf shouldn't work because Elfs usually are either ranged attack or magic attack types
  1. Instead of having one class that goes through a lot of if-else statements, you have multiple entities, which can accept or deny the effect, and then apply the effect on an intensity suitable to that entity type, you could also have some built-in limit for an effect on each entity separately, which might be helpful to fix entities judged to be too 'OP'.

If you have some experience with coding video games, having a lot of if-else statements is a bad habit, you should know that even games made with languages with faster run times than python struggle with a big amount of if-else statements, (Yandere Simulator is an example)

  1. Finally the class that used to handle the effect application, you could make it a base that contains how to trigger an effect appliance on the entity, and then make any object that can apply an effect inherit that part so you don't have to hard code it every time, you only inherit, initialize the effect information and function to the super class, it's easier this way.

Cons :

  1. The only downside is that you have to hard-code how each entity handles effects but that could be for the better

PLEASE, If you don't get something let me know.

@Grimmys
Copy link
Owner Author

Grimmys commented Sep 10, 2023

@IliyasDev sorry for the late answer, I was a bit busy working on other projects and needed to dig back in the code to be able to answer you correctly.

Assume you have multiple entity types, humanoids, goblins, elfs, animals... Having each class for these entities have its own Hash Map of effects is beneficial, because there might be some effects that should not work on that type, example:

You are right, the actual effect on the entity may depends on their characteristics in a foreseeable, but it is not right now.
They could not really be a specific dict/map by character race because races are not reflected as distinct classes in code, because character races are not directly affecting the behavior of the character: an elf and a dwarf are still both living playable entities. 😄
Class and race are only attributes.
To come back to the original point, tbh I didn't really think yet of the design for effects that may differ across Characters/Foes. It may be better maybe to just override the definition of Movable.set_alteration to verify if the given entity can be affected by the alteration according to its race, class or other criteria.

Also, different map for every possible race and classes in game seems to be too much code duplication to me, because in most cases, entities should be affected the same way by a given effect/alteration. It's only about exceptions.

Instead of having one class that goes through a lot of if-else statements, you have multiple entities, which can accept or deny the effect, and then apply the effect on an intensity suitable to that entity type, you could also have some built-in limit for an effect on each entity separately, which might be helpful to fix entities judged to be too 'OP'.

I think you are trying to explain what is polymorphism.
While I definitely agree with your point (this pattern should be followed as much as possible), I unfortunately don't think it's applicable to this situation, because of what I explained before: Goblins and Elfs are only different values for Character.race attribute, not different in-code classes.
Also, I'm a bit skeptical about the if-statements Yandere story, I heard about it quite a lot of times, but at the end, I think the issue was more about the lack of consistent design and basic logic rules not followed correctly (i.e., same conditions were checked again and again in the same frames). But I'm not an expert on Yandere's game code.
I just don't think if-else statements are that bad, at least not for perfs. Performance issues are more about wrong architecture and logical ordering decisions ; it's mainly redundant / unnecessary condition checks that can have an impact.

Finally the class that used to handle the effect application, you could make it a base that contains how to trigger an effect appliance on the entity, and then make any object that can apply an effect inherit that part so you don't have to hard code it every time, you only inherit, initialize the effect information and function to the super class, it's easier this way.

Not sure to get that part. Entities should not inherit from Effects.. Because they are not Effects.. Did you just mean that the application of an effect should be reversed? Like instead of having the method Effect.apply_on_ent(entity) there would be Entity.apply_effect(effect)?

Anyway, I definitely agree about the different pieces needing some refactoring: the init method of Effect could be generify to avoid having if-else statements sharing all a similar logic, and instead use a kind of map access to the details of the effect from the XML file.
Effect.apply_on_ent method also needs to be refactored, and if-else statements should probably be removed as well.

Lastly, I think documentation for both Alteration and Effect classes should be improved, because after reading them back, I noticed they could be quite confusing.

Please don't hesitate to argue more if you disagree with one of my point, I'm just sharing my personal feelings about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers text Relates to text visible in the application
Projects
None yet
Development

No branches or pull requests

2 participants