fheroes2
fheroes2 copied to clipboard
ihhub
Gray out combat spells when useless and show message without closing the spell book
This PR will gray out following spells as discussed in https://github.com/ihhub/fheroes2/discussions/4845:
- Bless and mass bless when player's army only has peasants.
- Curse and mass curse when opposing side only has peasants.
- Shield and mass shield when opposing side does not have any archers.
- Dragonslayer when opposing side does not have any dragons.
Any ideas @LeHerosInconnu for better messages for the user to inform why the spell cannot be used?
Hello @Kazhuu,
- Bless and mass bless when player's army only has peasants.
- Curse and mass curse when opposing side only has peasants.
- Shield and mass shield when opposing side does not have any archers.
- Dragonslayer when opposing side does not have any dragons.
Any ideas @LeHerosInconnu for better messages for the user to inform why the spell cannot be used?
I'll be making some proposals soon. :)
Hello @Kazhuu,
I'll be making some proposals soon. :)
So here are my proposals:
Player's army only has peasants, with "Bless" spells:
"Damage from peasant is constant, so there is no benefit to cast this spell."
Opposing side only has peasants, with "Curse" spells:
"Damage from peasant is constant, so there is no benefit to cast this spell."
It is also not possible to cast "Bless" spells and "Curse" spells on undead creatures.
Player's army only has undead, with "Bless" spells and "Curse" spells:
"Undead creatures cannot be affected by this spell."
It is also not possible to cast "Curse" spells on the Crusader.
Opposing side only has crusaders, with "Curse" spells:
"Crusader cannot be affected by this spell."
Opposing side only has crusaders and undead, with "Curse" spells:
"Crusader and undead creatures cannot be affected by this spell."
Opposing side only has crusaders and peasants, with "Curse" spells:
"Crusader cannot be affected by this spell. Damage from peasant is constant, so there is no benefit to cast this spell."
Or, better:
Crusader cannot be affected by this spell and damage from peasant is constant, so there is no benefit to cast this spell."
Opposing side does not have any ranged creatures, with "Shield" spells:
"Opposing hero's army does not include any ranged creatures, so there is no benefit to cast this spell."
Opposing side does not have any dragons, with "Dragon Slayer" spell:
"Opposing hero's army does not include any dragons, so there is no benefit to cast this spell."
Edit 2023/06/25.
I didn't mention certain cases.
Opposing side only has peasants and undead, with "Bless" spells and "Curse" spells:
"Undead creatures cannot be affected by this spell and damage from peasant is constant, so there is no benefit to cast this spell."
Opposing side only has crusaders, peasants and undead with "Curse" spells:
"Crusader and undead creatures cannot be affected by this spell and damage from peasant is constant, so there is no benefit to cast this spell."
@LeHerosInconnu sorry for the delay. Now I added the strings you asked. Can you test this to verify it's working like you wanted?
Hello @Kazhuu,
@LeHerosInconnu sorry for the delay. Now I added the strings you asked. Can you test this to verify it's working like you wanted?
I'll test it when I have some time, maybe next week. :)
Just to be clear, the spell Shield doesn't affect spells such as Magic Arrow?
Hi @Mr-Bajs
Just to be clear, the spell Shield doesn't affect spells such as Magic Arrow?
No, it shouldn't affect any magic damage. Moreover, it shouldn't affect the damage from the castle towers, which is currently not the case in fheroes2 and should be fixed by #7466.
Hello @Kazhuu,
Hi @LeHerosInconnu any news on this one?
I don't have too much time at the moment, but don't worry it's still on my to-do list. :)
Hello @Kazhuu and @ihhub,
Hi @LeHerosInconnu , please provide your feedback for these changes.
With bless and curse spells, for the strictly identical cases presented in the original post, the display of messages works correctly.
- Bless and mass bless when player's army only has peasants.
- Curse and mass curse when opposing side only has peasants.
Here are my observations with the shield and dragon slayer spells.
- Shield and mass shield when opposing side does not have any archers.
- Dragonslayer when opposing side does not have any dragons.
In multiplayer mode, two human players, green and blue, blue player's hero attacking:
- The defending green hero can cast shield and mass shield even if there is no ranged creature in the opposing blue player's army, as long as he has a ranged creature in his army.
- The defending green hero cannot cast the shield and mass shield spells even if there is a ranged creature in the opposing blue player's army as long as he does not have a ranged creature in his army.
- The defending green hero can cast the dragon slayer spell even if there is no dragon in the opposing blue player's army as long as he has a dragon in his army.
Also, the message should be adapted to the creatures still present when the spell is cast, and not to the creatures present at the start of the combat.
In the example, with curse spell, the peasants have been decimated and the message displays a description as if they are still alive and cannot be affected.
The message should adapt to the creatures still present, but not to those decimated.
This PR covers only the strictly identical cases presented in the original post. There are other cases to be taken into consideration.
Here are the test scenario and the save files: Grayed combat spells 01.zip
Hello @LeHerosInconnu thanks for the feedback! I'll take a look at the code again and fix these soon. Could you elaborate what you mean by this?
There are other cases to be taken into consideration.
What other cases as well we should cover to gray out spells? Maybe if they are not related to here, I can do these in a separate PR.
Resurrection and True Ressurection, when there are no dead troops in non-elemental stacks: "There are no dead troops in your army, the spell is useless... for now."
Animate Dead when there are no destroyed undead creatures in stacks: "There are no fallen undead troops in your army, the spell is useless... for now."
Death Ripple and Death Wave spells, when all troops in enemy army are undead: "This spell will cause death and decay to your army and do nothing to your enemy!"
Holy Word and Holy Shout when there are no undead troops in enemy army: "This spell will devastate your undead army and do nothing to your enemy!"
Antimagic spell when enemy hero has 0 mana and there are no spellcasting creatures in his army: "Enemy hero can`t cast any spells and none of his creatures can cast spells on attack. This spell is useless for now."
Earthquake spell outside siege battles: "There are no castle walls to destroy using this spell"
Earthquake spell during castle defending: "This spell will devastate your castle walls!"
Summon Elementals spells when other summoned elementals are present on battlefield: "Those elemtal beings will kill each other, if not completely controlled."
Dispel and Mass Dispel, when there are no spells affecting your or enemy troops: "Currently there are no magic effects affecting anyone on this battlefield, this spell is useless for now."
Shield and Mass Shield spells when sieging enemy castle when only shooting enemies are arrow towers: "Castle towers use enchanted crossbow bolts that can penetrate any kind of defense, physical or magical, including effect of this spell"
Hello @Kazhuu,
Hello @LeHerosInconnu thanks for the feedback! I'll take a look at the code again and fix these soon. Could you elaborate what you mean by this?
There are other cases to be taken into consideration.
What other cases as well we should cover to gray out spells? Maybe if they are not related to here, I can do these in a separate PR.
In the game, as soon as the conditions are met, the message "That spell will affect no one!" is displayed. All these conditions have already been anticipated, since the message is displayed. This affects all combat spells, depending on the case (Armageddon, Resurrection True, etc.) The place where the message is displayed should be moved from the battlefield, after the hero has cast the spell, to the magic book, at the moment when the hero attempts to cast the greyed-out spell (depending on the conditions already anticipated). All this is indicated in the https://github.com/ihhub/fheroes2/discussions/4845 thread. The spells bless, curse, etc. are special cases in that they can be cast even if they have no effect (e.g. the bless spell on peasants). This PR can focus only on the four cases indicated in the original post, that's okay.
Resurrection and True Ressurection, when there are no dead troops in non-elemental stacks: "There are no dead troops in your army, the spell is useless... for now."
Animate Dead when there are no destroyed undead creatures in stacks: "There are no fallen undead troops in your army, the spell is useless... for now."
Death Ripple and Death Wave spells, when all troops in enemy army are undead: "This spell will cause death and decay to your army and do nothing to your enemy!"
Holy Word and Holy Shout when there are no undead troops in enemy army: "This spell will devastate your undead army and do nothing to your enemy!"
Antimagic spell when enemy hero has 0 mana and there are no spellcasting creatures in his army: "Enemy hero can`t cast any spells and none of his creatures can cast spells on attack. This spell is useless for now."
Earthquake spell outside siege battles: "There are no castle walls to destroy using this spell"
Earthquake spell during castle defending: "This spell will devastate your castle walls!"
Summon Elementals spells when other summoned elementals are present on battlefield: "Those elemtal beings will kill each other, if not completely controlled."
Dispel and Mass Dispel, when there are no spells affecting your or enemy troops: "Currently there are no magic effects affecting anyone on this battlefield, this spell is useless for now."
Shield and Mass Shield spells when sieging enemy castle when only shooting enemies are arrow towers: "Castle towers use enchanted crossbow bolts that can penetrate any kind of defense, physical or magical, including effect of this spell"
Could all these cases be listed on discussion https://github.com/ihhub/fheroes2/discussions/4845. This way they don't get lost in PR comments after this PR is merged. Also if there are more cases that are not listed here. It would be nice to have them on there as well.
Hello @Kazhuu,
@LeHerosInconnu I fixed the issues you mentioned. Could please test again?
I'll look at it over the course of next week.
Hello @Kazhuu,
Hello @Kazhuu,
@LeHerosInconnu I fixed the issues you mentioned. Could please test again?
I'll look at it over the course of next week.
Here are my observations with the last change.
In multiplayer mode, two human players, green and blue, blue player's hero attacking:
The attacking blue hero no longer has a "living" undead troop in his ranks, but the curse spell description indicates undead.
The defending green hero can cast the curse spell on the peasants, although the spell will have no effect.
The attacking blue hero can cast the curse spell on the peasants, although the spell will have no effect.
The attacking blue hero can cast the curse spell, the message "That spell will have no effect!" is displayed on the battlefield instead of the curse spells being grayed out.
The defending green hero can cast the curse spell, the message "That spell will have no effect!" is displayed on the battlefield instead of the curse spells being grayed out.
The attacking blue hero can cast the bless spell on the peasants, although the spell will have no effect.
The defending green hero can cast the bless spell on the peasants, although the spell will have no effect.
The attacking blue hero can cast the bless spell, the message "That spell will have no effect!" is displayed on the battlefield instead of the bless spells being grayed out.
The defending green hero can cast the bless spell, the message "That spell will have no effect!" is displayed on the battlefield instead of the bless spells being grayed out.
Hi @LeHerosInconnu thank you a lot for your feedback and testing! Apparently I have not touched the functionality for these outside of the spell book usage. I'll add code to cover these cases as well and ping here when done.
Hi @Kazhuu and @LeHerosInconnu , what's the status of this pull request? I suggest if the cannot do everything at once focus on one spell at the time.
Hello @ihhub! I still plan to work on this but currently being busy with work. This PR already has spell graying out functionality, but missing checks when the spell can be cast, but not on certain units. So I need to implement the checks for those cases where some units should display cross cursor indicating that spell cannot be used for that unit. I'll have more time to work on in coming weeks :+1:
We shall proceed with what changes we have to do not delay this pull request any longer.
Hi @ihhub. I updated the headers and fixed IWYU problems as well. Can you take a look?
Shouldn't this also include the spell immunity for the living dragons (Green, Red and Black)?
Shouldn't this also include the spell immunity for the living dragons (Green, Red and Black)?
I think it's better to merge this first and work on remaining ones in a separate PR.
Shouldn't this also include the spell immunity for the living dragons (Green, Red and Black)?
I think it's better to merge this first and work on remaining ones in a separate PR.
I think specificly dragons are always immune to spells so they should be included in this.
Example Crusaders and undead creatures cannot be affected by this spell and peasants have no range of damage, so this spell will have no effect
This text doesn't make sence if the player meets or has only Crusaders, peasants, undead and dragons. It implies that the spells should work against atleast the dragons. It's a bit confusing and i guess this would be reworked anyway to include the dragons effects?
It should work like something like this?
Crusaders, living dragons and undead creatures cannot be affected by this spell and peasants have no range of damage, so this spell will have no effect
I rephrased the texts like you asked @zenseii. Can you check?
The texts look good now 👍 I am however considering the points @Mr-Bajs brought up, not necessarily because the fact about dragons should be mentioned too, but rather about the principle of listing every cause. I haven't come to a conclusion since there are clear pros and cons of either way. This is also not why my review of this PR was requested.
Nevertheless, the graying out of the spells is a good change.
Sorry @Mr-Bajs. At first I didn't understand what you meant. And yes they should be included here as well. Are there any other causes that we are missing? As mentioned adding every cause also adds more complexity to the code if this is required. However at this point adding dragons to this is not a big work anymore if needed. Whatever fits best this projects goals, I'll do the changes :+1:
I think they should be as detailed as possible when implemented. It's a rather nice feature that also teaches the player the effects of the spells. However there is no rush to implement them and i think there are about 38 different combat spells in heroes2. I that the current case with dragons, pesants, crusaders and undead are as complex as they can be at most.
Most other cases just slighty looking through the spells and immunites they 2 factor so consider. Exept for phoenixes, living dragons and elementals that have overlapping immunities.