ihhub / fheroes2

fheroes2 is a recreation of Heroes of Might and Magic II game engine.
https://ihhub.github.io/fheroes2/
GNU General Public License v2.0
2.68k stars 373 forks source link

A message, that player was defeated, appears only after AI turn is over #2625

Open Branikolog opened 3 years ago

Branikolog commented 3 years ago

During an AI turn, when player loses a hero, whose name is in scenario loss condition list, the game doesn't stop, until AI turn is over. A message, that the match is lost should appear at the moment hero was defeated or fled from a battle. Also, hero list doesn't refresh currently, until player's turn starts. We need to show hero vanishing animation, delete his portrait from the hero list and then show the message.

Branikolog commented 3 years ago

Save file. Lose Hero.zip

oleg-derevenetz commented 3 years ago

Hi @Branikolog I suppose that this PR is already was fixed one way or another. I wasn't able to load your save (it was produced by the version of the game that isn't supported anymore), but I tried to start the map "Merry Men", attack group of archers near village by Robin the Knight and retreat at the first turn. I immediately got the message "You have lost the hero Robin. Your quest is over." and immediately exited to the main menu, red player (AI-controlled) wasn't get the turn.

Branikolog commented 3 years ago

Hi, @oleg-derevenetz The issue is still on. AI needs to attack special hero and at the moment the hero is defeated we should watch a message. AI defeats hero.zip Also, we have a certain problems with this special loss condition. While playing a multiplayer the game doesn't stop, after the hero was defeated... I believe in future we have to set special loss condition for each player, as the original editor allows to set only a single hero to be the condition of win\loss of a scenario.

oleg-derevenetz commented 3 years ago

Hi @Branikolog

AI needs to attack special hero and at the moment the hero is defeated we should watch a message.

Yeah, it seems that AI-related code doesn't check win/loss conditions after each hero action, as HumanTurn() currently does.

While playing a multiplayer the game doesn't stop, after the hero was defeated...

Currently in a multiplayer mode special win/loss conditions of a map are not checked at all (I don't know are they taken into account in OG), you should take all opponent's castles and kill all heroes (or wait for 7 days after last opponent's castle was taken), so in multiplayer mode there are always WINS_ALL and LOSS_ALL conditions are active.

Branikolog commented 3 years ago

Currently in a multiplayer mode special win/loss conditions of a map are not checked at all (I don't know are they taken into account in OG), you should take all opponent's castles and kill all heroes (or wait for 7 days after last opponent's castle was taken), so in multiplayer mode there are always WINS_ALL and LOSS_ALL conditions are active.

I see. In the OG if player loses hero - the game is over. Even in multiplayer... But I'm not sure, if it's correct behaviour in this case...

ihhub commented 1 year ago

Bumping the priority for this issue and also marking it as a bug.

dood-apo commented 8 months ago

Actually we might consider handling the vanquish/game-loss logic on the beginning of the turns exclusively. This would mean that you get to watch AI's turns before getting a game loss info, but then this behavior could be applied to multiplayer also and everything would be consistent. No matter if it was a special hero lost or a week without a castle has passed, the behavior could be the same for single/multi player. See my comment in #7091

zenseii commented 8 months ago

The annoying part about having to watch a player's turn to completion after having been defeated is exactly that. If a player defeats you in the beginning of a long turn, then you have to wait for the end of that turn, and then maybe for other players' turns too, before you're told you've been defeated, get the defeat animation, and can either start a new game or reload a save.

dood-apo commented 8 months ago

Right - for single player it makes sense. But for multiplayer it is questionable. Your presence might influence the decisions of the players before you in the turn order, then you vanquish because you have been homeless for too long and the players that go after you in the turn order do not have to plan for you this turn. Maybe it's fine though. I could take a look, but would need to have a confirmation what is the desired outcome with both single player and multiplayer in mind.

oleg-derevenetz commented 8 months ago

Actually we might consider handling the vanquish/game-loss logic on the beginning of the turns exclusively. This would mean that you get to watch AI's turns before getting a game loss info, but then this behavior could be applied to multiplayer also and everything would be consistent.

Again, turns and days are different things, please do not mix them. If you have 3 players, they will perform 3 turns (each of them will perform one turn) in just one day. At the moment, checking whether someone is winning or losing (due to the map win/loss conditions) is performed right after each hero's action for human players and once a turn for AI players (actually, it's a little more complicated, but for simplicity, let it be so). However, when a player has lost all of his castles, but not the heroes, he is given 7 days until he is considered defeated, his kingdom will be still alive on day 7 even after completion of his turn. I don't see any point in changing this logic, the only thing that can be improved here is to speed up the win/loss condition check for AI players (perform it after each hero's action, just like it is done for human-controlled players), but I think if this is a "bug", it's just a minor one and does not affect the gameplay in any way, except for cosmetic inconvenience.

dood-apo commented 8 months ago

Well, I think that even if something is a minor inconvenience it doesn't mean there is no point of addressing it. I think that first and foremost the game-loss logic should be consistent across single and multiplayer modes. How exactly it should look like to be functional for multiplayer and not annoying at the same time for both modes is another matter.

When I was reproducing #7091 on my end (which seems like a bug to me - you're told you're done, but you can be challenged to a fight by the next player in line), I've also noticed that the warning that you've lost your castles and you better find one is issued at the end of your first homeless turn, not at the beginning. Even a more minor inconvenience I guess.

Regarding mixing days and turns - you are correct I didn't phrase myself very well. What I meant and should've said instead was that "we might consider handling the vanquish/game-loss logic on the beginning of days and give the info dialog about that fact to every player on the beginning of their turn in that day".

oleg-derevenetz commented 8 months ago

which seems like a bug to me - you're told you're done, but you can be challenged to a fight by the next player in line

Warning message clearly says that you have 7 "days", not "turns". It seems to you that this is a bug, but I believe that this is just the right behavior in accordance with what was promised. Nothing more than a difference of opinion.

I've also noticed that the warning that you've lost your castles and you better find one is issued at the end of your first homeless turn, not at the beginning. Even a more minor inconvenience I guess.

This behavior was replicated from the original game:

https://github.com/ihhub/fheroes2/assets/32623900/4150ed7e-b552-4f52-8fdb-aca0a2661183

we might consider handling the vanquish/game-loss logic on the beginning of days and give the info dialog about that fact to every player on the beginning of their turn in that day

If a human player already lose the scenario, there's no need to make him wait until the next day to report this. This may be confusing: "I lost my special hero but the game still continues as if nothing had happened". And maybe even worse: "I lost my special hero, but I managed to re-hire it in my castle, so let's just pretend that nothing happened".

dood-apo commented 8 months ago

Warning message clearly says that you have 7 "days", not "turns". It seems to you that this is a bug, but I believe that this is just the right behavior in accordance with what was promised. Nothing more than a difference of opinion.

You definitely have a point there. However, wouldn't you agree that it is a bit weird to lose and still play a battle on the next player's turn?

I've also noticed that the warning that you've lost your castles and you better find one is issued at the end of your first homeless turn, not at the beginning. Even a more minor inconvenience I guess.

This behavior was replicated from the original game

Yeah, I thought it was the same in OG.

And maybe even worse: "I lost my special hero, but I managed to re-hire it in my castle, so let's just pretend that nothing happened".

Ok, this right here convinced me that postponing game-loss stuff is a bad idea!

oleg-derevenetz commented 8 months ago

You definitely have a point there. However, wouldn't you agree that it is a bit weird to lose and still play a battle on the next player's turn?

Heh, well, the mechanics of the turn-based game are generally somewhat strange in this regard: a strong hero approaches you with the intention of stuffing your face, but you can't immediately run away :) In "real life", you would run away from him, especially if you have a bigger reserve of movement points than him, but... not in the turn-based game. So no, I don't feel anything particularly strange about it... considering all the circumstances.