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

If only non-defending units are left defending a city after combat, it should be destroyed #395

Open
QuintillusCFC opened this issue Feb 11, 2023 · 2 comments
Labels
bug Something isn't working refactor Refactor something to make it better

Comments

@QuintillusCFC
Copy link
Member

This is a follow-up to #286

Currently, if a non-combat unit is the only defending unit when a rival unit attempts to move into the city, it is destroyed (eventually it will be captured).

However, if there is exactly one defending unit, and it is defeated, the city is not destroyed. A lone Catapult may prevent capture, for example.

This appears to be due to the interaction of these code blocks:

	// If the enemy was defeated, check if there is another enemy on the tile. If so we can't complete the move
	// but still pay one movement point for the combat.
	else if (combatResult == CombatResult.DefenderKilled || combatResult == CombatResult.DefenderRetreated) {
		if (!unit.CanEnterTile(newLoc, false)) {
			unit.movementPoints.onUnitMove(1);
			return true;
		}

And and CanEnterTile method in MapUnitExtensions (excerpt):

		// Check for units belonging to other civs
		foreach (MapUnit other in tile.unitsOnTile)
			if (other.owner != unit.owner) {
				if (!other.owner.IsAtPeaceWith(unit.owner))
					return allowCombat;
				else
					return false;
			}

It is returning false because allowCombat is false since combat already ended.

A better way of doing this might be to utilize TileExtension's FindTopDefender method (which uses MapUnitExtension's canDefendAgainst method, and if there are no defenders, the unit can move in.

But CanDefendAgainst will also need to be enhanced to account for non-defense-capable units.

Adding the refactor tag since there is some duplication of logic between methods, and the area can probably be simplified a bit.

@QuintillusCFC QuintillusCFC added bug Something isn't working refactor Refactor something to make it better labels Feb 11, 2023
@QuintillusCFC
Copy link
Member Author

Random seed: 1893199352
Commit: 9b0998b
Turn: 56

A Veteran Barbarian Warrior will attack Babylon and win, with one hitpoint left. Right-click on Babylon and you can see in the console that only one Catapult remains.

@TomWerner
Copy link
Contributor

I've got a slightly easier way to reproduce this issue, we can modify the default save to put an enemy city right next to our start, and give ourselves a warrior.

If you start the game via "new game", wait one turn, and then attack the American city, either the worker or the scout will be killed, but the city won't be taken - the second non-defending unit remains.

https://github.com/C7-Game/Prototype/compare/twrner/patch-for-non-defending-units?expand=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor Refactor something to make it better
Projects
None yet
Development

No branches or pull requests

2 participants