Skip to content

Comments

don't fail map if no NPCs are visible, fixes taxi-04 with 3 transfers#129

Merged
mgerhardy merged 3 commits intomgerhardy:masterfrom
cryham:fix-npc-count-maps-fail
Mar 9, 2025
Merged

don't fail map if no NPCs are visible, fixes taxi-04 with 3 transfers#129
mgerhardy merged 3 commits intomgerhardy:masterfrom
cryham:fix-npc-count-maps-fail

Conversation

@cryham
Copy link
Contributor

@cryham cryham commented Mar 7, 2025

This fixes taxi-04 properly and all other taxi map issues: if NPCs arent visible (hidden in caves) map will fail, with no reason, this was a bug.
Feel free to restore taxi-04, it will work now, with 3 transfers as before. 5cac237

if (_players.empty())
return true;

/* old, bad
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the commented code. if it is wrong is has no real value here. Even though I wonder how can we lose such a map now?

Copy link
Contributor Author

@cryham cryham Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done
Not sure. But eventually users will click restart, as usual.

@cryham
Copy link
Contributor Author

cryham commented Mar 9, 2025

Updated

@mgerhardy
Copy link
Owner

I think I remember again - this was here to ensure that the player doesn't just kill the npcs without transporting them. And if they killed enough npcs they are not able to win the match anymore. So either you are idling in the map without knowing what to do, or we have to find a way to tell the players that they have lost the match. So simply removing it might not be the best option here.

@cryham
Copy link
Contributor Author

cryham commented Mar 9, 2025

Well the old check was bad. Since it failed a map when all NPCs were in caves. I could easily trigger this.
So we should probably check how many NPCs were killed instead then.
I thought if we do it like now, users will simply restart a map eventually.

@cryham cryham force-pushed the fix-npc-count-maps-fail branch from b3e3390 to ca20935 Compare March 9, 2025 20:45
@cryham
Copy link
Contributor Author

cryham commented Mar 9, 2025

Rewrote it to count and fail map if killed all NPCs.

@mgerhardy mgerhardy merged commit 6fbe997 into mgerhardy:master Mar 9, 2025
3 of 4 checks passed
@cryham cryham deleted the fix-npc-count-maps-fail branch March 9, 2025 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants