Skip to content

Mechanic for the City destroy #13742

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Conversation

PhiRite
Copy link
Contributor

@PhiRite PhiRite commented Jul 29, 2025

Removed the one that allows a unit to destroy all types of cities so now its just 1 unique that allows a unit to destroy non capital non holy cities.

however i believe things like setting mod option to allow razing city may make it work? not sure though.

} else
} else if (attacker.unit.hasUnique(UniqueType.CanDestroyCities)) {
if (defender.city.isCapital()) {
conquerCity(defender.city, attacker) // Capital is captured
Copy link
Owner

Choose a reason for hiding this comment

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

... What?
If a range unit has 'can destroy cities' then it can capture capital cities?

Copy link
Owner

Choose a reason for hiding this comment

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

Also if a unit has can destroy and also can't capture, it doesn't activate?

Basically, this should be a separate check, prior to "can capture"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ye i basically decided to just use the capture block since originally it was meant to be just a way to allow to destroy all cities so it wouldn't matter if the unit was ranged or not, I will change that.

@PhiRite
Copy link
Contributor Author

PhiRite commented Jul 30, 2025

Changed it to:
works outside of the capture logic
melee units are the only one to be able to capture capital cities, but both ranged and melee should be able to destroy non capital

if (defender.isDefeated() && defender is CityCombatant && attacker is MapUnitCombatant
&& attacker.unit.hasUnique(UniqueType.CanDestroyCities, checkCivInfoUniques = true)) {
if (defender.city.isCapital() && attacker.isMelee()) {
conquerCity(defender.city, attacker) // Capital is captured by only melee units
Copy link
Owner

Choose a reason for hiding this comment

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

should be "do nothing" because we capture it below anyway

Copy link
Owner

Choose a reason for hiding this comment

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

In fact the melee part isn't interesting
The "and not capital" should be part of the initial large if, and then we don't have any nesting at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just made it into this

if (defender.isDefeated() && defender is CityCombatant && attacker is MapUnitCombatant
&& attacker.unit.hasUnique(UniqueType.CanDestroyCities, checkCivInfoUniques = true) && !defender.city.isCapital()) {
defender.city.destroyCity()
}

shorter and should work per your comment

removed stuff


// allow any unit to destory cities instead of capturing them unless its a Capital City, also allows non melee units to destroy cities but only melee can capture capital cities
CanDestroyCities("Can destroy non capital cities", UniqueTarget.Unit, UniqueTarget.Global,
Copy link
Owner

Choose a reason for hiding this comment

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

Should be cityFilter cities

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by this? im not sure what to change since making it be able to destroy cityFilter means a unit should be able to then destroy even cities that they own? tho I suppose i dont think a unit can just attack your own stuff? Still not sure i understand though.

Copy link
Contributor Author

@PhiRite PhiRite Jul 30, 2025

Choose a reason for hiding this comment

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

Actually the more i think about it changing it to cityfilter could work? im still not sure i completely understand what you mean but making it cityfilter could allow it to be more flexible for modders but im not sure if that will even work.

Copy link
Owner

Choose a reason for hiding this comment

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

"Can destroy [cityFilter] cities", and in the documentation add "Cannot destroy capitals" - that's a constant throughout the game and not something special to this unique

PhiRite added 2 commits August 1, 2025 18:45
still might be broken? doesnt destroy city states as of now
@PhiRite
Copy link
Contributor Author

PhiRite commented Aug 1, 2025

updated it, its will probably need to be changed to make it way better but as of right now it seems to not be able to destroy city states even using "Can destroy [in City-State cities] cities".

if (defender.isDefeated() && defender is CityCombatant && attacker is MapUnitCombatant) {
if (!defender.city.isCapital()) {
val destroyFilters = attacker.unit.getMatchingUniques(UniqueType.CanDestroyCities).flatMap { unique ->
unique.params[0]?.let { sequenceOf(it) } ?: emptySequence()
Copy link
Owner

Choose a reason for hiding this comment

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

Param[0] must exist, or else it wouldn't match the unique, so this should just be .map { unique.params[0] }

PhiRite added 2 commits August 4, 2025 16:07
test check
removed extra bracket
@PhiRite
Copy link
Contributor Author

PhiRite commented Aug 4, 2025

i havent completely checked but is it normal that this doesnt destroy city states even with it correctly set?

@SomeTroglodyte
Copy link
Collaborator

City state cities are still capitals1 - Maybe that's why?

Footnotes

  1. except for the extremely rare case where they conquer another city by accident, which always causes them to raze it (so they stay city-states), then lose their capital before the raze is complete...

@PhiRite
Copy link
Contributor Author

PhiRite commented Aug 4, 2025

Should destroying them be not allowed then? since capitals are supposed to be immune? kinda wanted it to be destroyable but still.

@PhiRite
Copy link
Contributor Author

PhiRite commented Aug 6, 2025

so from what i understand this should be correct since city states should be immune as well since its capital? i believe its also same logic as civ 5 with not being able to raze city states?

@PhiRite
Copy link
Contributor Author

PhiRite commented Aug 11, 2025

I updated my fork to see if nay of the newer updates would have broken stuff.

@PhiRite
Copy link
Contributor Author

PhiRite commented Aug 11, 2025

this should now be good as to my understanding city state are counted as a capital?

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.

3 participants