Ticket #45225

civ2civ3 naval units fix

Eröffnet am: 2022-07-25 05:42 Letztes Update: 2022-09-01 20:02

Auswertung:
Verantwortlicher:
Typ:
Status:
Geschlossen
Komponente:
Meilenstein:
Priorität:
5 - Mittel
Schweregrad:
5 - Mittel
Lösung:
Gefixt
Datei:
2

Details

Fixes to civ2civ3 naval units -

Make Triremes, Caravels, Galleons and Transports non-military units with uk_happy=0 and attack=0 so there will always be a naval unit that can transport a caravan to an overseas foreign city during peacetime.

Submarines and Carriers should have uk_happy=1. Also make Carrier a FieldUnit.

Ticket-Verlauf (3/22 Historien)

2022-07-25 05:42 Aktualisiert von: ddeanbrown
  • New Ticket "civ2civ3 naval units fix" created
2022-07-25 05:42 Aktualisiert von: ddeanbrown
  • File civ2civ3-naval-units-master.patch (File ID: 9923) is attached
2022-07-25 05:42 Aktualisiert von: ddeanbrown
  • File civ2civ3-naval-units-S3_1.patch (File ID: 9924) is attached
2022-07-25 05:43 Aktualisiert von: ddeanbrown
Kommentar

Patches for master and S3_1 attached for review

2022-07-25 17:58 Aktualisiert von: alienvalkyrie
Kommentar

In the S3_1 patch, the "NonMil" is added to the roles for the Galleon, which causes ruleset load failure.
That seems to be the only difference to the master patch; so we might not even need a separate S3_1 patch (but rather use the same patch for both branches) ~> could you confirm that?

If your patches include metadata (author, commit message etc.), like those created by git format-patch (after committing your changes, use e.g. git format-patch origin/master), then after merging, the commits are correctly attributed to you. When you do, make sure the commit message refers to this ticket, e.g. with a "See osdn#45225" line; if you've already committed them, you can use git commit --amend to change the message after the fact.
(If you'd rather not to do that, I'll credit you in the commit message instead.)

2022-07-26 03:14 Aktualisiert von: ddeanbrown
  • File civ2civ3-naval-units-master.patch (File ID: 9923) is deleted
2022-07-26 03:15 Aktualisiert von: ddeanbrown
  • File civ2civ3-naval-units-S3_1.patch (File ID: 9924) is deleted
2022-07-26 03:18 Aktualisiert von: ddeanbrown
Kommentar

2nd try with corrections. Changes are the same to master and S3_1 so this patch should work for both.

2022-07-26 03:48 Aktualisiert von: alienvalkyrie
  • Verantwortlicher Update from (Keine) to alienvalkyrie
  • Lösung Update from Keine to Accepted
  • Meilenstein Update from (Keine) to 3.1.0
2022-07-27 14:00 Aktualisiert von: None
Kommentar

Just commenting that the tireme and caravel (as of 3.0) both have attack values of 1 in the civ2civ3 ruleset, somewhat justifying their unhappiness requirement (though I personally haven't seen AI attack with them in games I played). It might be best to set their attack to 0.

If the carrier is being changed into a field unit, I think it should have a non-zero attack value to justify it being a field unit (in the classic ruleset, it was set to 1).

2022-07-27 18:21 Aktualisiert von: alienvalkyrie
Kommentar

Reply To (Anonymous)

Just commenting that the tireme and caravel (as of 3.0) both have attack values of 1 in the civ2civ3 ruleset, somewhat justifying their unhappiness requirement (though I personally haven't seen AI attack with them in games I played). It might be best to set their attack to 0.

That's already part of the patch.

If the carrier is being changed into a field unit, I think it should have a non-zero attack value to justify it being a field unit (in the classic ruleset, it was set to 1).

I take it you're saying the patch shouldn't be merged in its current state? I'll delay pushing it until this question is resolved.

I don't exactly agree. For one, all carrier-based units except one (the AWACS) are combat units, and putting them on a carrier generally implies offensive use, so I think a carrier is plenty aggressive enough to justify being a field unit, even without an attack value – but ultimately that's a balancing issue, which I don't feel qualified to make judgment calls on.
More importantly however, at the point in the game where carriers come in, a small-but-nonzero attack power isn't gonna do anything significant (except make the AI / auto-attack server advisors do stupid things, or maybe get in a desperado attack against something that's already almost dead?), and an attack power significant enough to do anything would probably be overpowered.
There's also an argument to be made that a carrier's offensive capabilities should come entirely from the planes it has on board as a matter of preference.

2022-07-27 20:27 Aktualisiert von: None
Kommentar

Reply To alienvalkyrie

Reply To (Anonymous)

Just commenting that the tireme and caravel (as of 3.0) both have attack values of 1 in the civ2civ3 ruleset, somewhat justifying their unhappiness requirement (though I personally haven't seen AI attack with them in games I played). It might be best to set their attack to 0.

That's already part of the patch.

Great, I must have missed that detail.

If the carrier is being changed into a field unit, I think it should have a non-zero attack value to justify it being a field unit (in the classic ruleset, it was set to 1).

I take it you're saying the patch shouldn't be merged in its current state? I'll delay pushing it until this question is resolved. I don't exactly agree. For one, all carrier-based units except one (the AWACS) are combat units, and putting them on a carrier generally implies offensive use, so I think a carrier is plenty aggressive enough to justify being a field unit, even without an attack value – but ultimately that's a balancing issue, which I don't feel qualified to make judgment calls on.
More importantly however, at the point in the game where carriers come in, a small-but-nonzero attack power isn't gonna do anything significant (except make the AI / auto-attack server advisors do stupid things, or maybe get in a desperado attack against something that's already almost dead?), and an attack power significant enough to do anything would probably be overpowered.
There's also an argument to be made that a carrier's offensive capabilities should come entirely from the planes it has on board as a matter of preference.

I understand a fully loaded carrier is a literal war machine (and I have used it in games to clean up), but the bombers, helicopters, cruise missiles and nukes (if it carries any of those) are already field units. Not to mention in the civ2civ3 ruleset, it can carry standard land units (but not big land units which I personally find odd considering a carrier should have the capability to store those). When its out at open sea which it will be a lot when someone has that much units, the owning civilization is already going to take a massive hit in unhappiness that unless the carrier is given the capability to attack, I don't see the point in making it a field unit when most of units it carries are already field units or will be outside the civilization's borders for the majority of the carrier's shelf life (since under civ2civ3 rules, only adjacent to city oceanic tiles are under borders).

2022-07-28 09:41 Aktualisiert von: ddeanbrown
Kommentar

I don't have a problem with the carrier being a field unit with attack = 0.

I would prefer it to not carry land units (except maybe Marines), that's more realistic. Making that change is also a bit of mission creep for this ticket, but I would be OK with it. Of the standard supplied rulesets, currently only civ2civ3 and sandbox allow transporting land units, the others do not. SIM30 and mp2-caravel do not, LTT does.

2022-07-28 09:47 Aktualisiert von: ddeanbrown
Kommentar

Just realized that in my patch I changed the helptext for the Trireme - with the change to attack=0 it was incorrect. That helptext is a translated string, so new translations needed. Should be easy, it's just deleting some words. Maybe need a new ticket for that task?

2022-07-28 11:00 Aktualisiert von: cazfi
Kommentar

Reply To ddeanbrown

Of the standard supplied rulesets, currently only civ2civ3 and sandbox

When doing changes to civ2civ3, one usually needs to either do the same change to sandbox, or to document the difference in README.sandbox (from the point of how sandbox differs from civ2civ3, even though it was civ2civ3 that changed)

2022-08-06 22:32 Aktualisiert von: cazfi
Kommentar

Reply To ddeanbrown

I changed the helptext for the Trireme - with the change to attack=0 it was incorrect ... Maybe need a new ticket for that task?

It's logically part of the attack = 0 change, isn't it? So belongs to this ticket.

2022-08-07 17:42 Aktualisiert von: alienvalkyrie
  • Lösung Update from Accepted to Keine
Kommentar

The way I see it, there are two things still blocking this patch:

  • Figuring out how it should relate to sandbox, i.e. either porting the changes there, or documenting the difference
  • Figuring out the carrier field unit thing, i.e. whether it should (a) not be a field unit (current behavior), (b) be a field unit with no attack power (current patch), or (c) be a field unit with some positive attack power (a bit like the classic ruleset)

For the former, I'd tend toward porting the changes to the sandbox ruleset – unless the old behavior is necessary for some other use of new features in the sandbox ruleset (which, without checking, sounds unlikely).

For the latter, the safest compromise would likely be (a), i.e. to leave the carrier as-is, or perhaps to give it uk_happy without being a field unit; (b) has already gotten resistance, and (c) doesn't really seem better IMO – attacking anything (except maybe a sub?) with a carrier doesn't seem like something that would be sensible anyhow.

2022-08-29 23:39 Aktualisiert von: cazfi
Kommentar

It would be good to get this go forward, so such a change to default ruleset would be exposed to testing already.

For the sandbox part, just default to making same changes there - there's no reason given not to have civ2civ3 and sandbox similar in this respect.
If we have no agreement on the carrier issue, maybe leave that part out from this patch (so the issue won't block this any further), and open a separate ticket for it. Or does someone consider it such an elementary part of this overall change, that the other parts should not go in either, without it?

2022-08-31 02:50 Aktualisiert von: alienvalkyrie
  • Lösung Update from Keine to Accepted
Kommentar

New patch with the following changes relative to the previous one:

  • Reverted the carrier change
  • Moved the helptext part about not being able to attack, only defend itself from the galleon to the trireme
  • Ported the changes to sandbox
2022-09-01 20:02 Aktualisiert von: alienvalkyrie
  • Status Update from Offen to Geschlossen
  • Lösung Update from Accepted to Gefixt
Kommentar

Note: I'll leave opening a separate ticket for the Carrier question to whomever feels there still needs to be a change.

Bearbeiten

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Anmelden