Thalassicus / cep-bnw

Civ V Communitas Expansion Pack
32 stars 22 forks source link

Swedish Allotment Trait issues #189

Closed EricB1 closed 10 years ago

EricB1 commented 10 years ago

There was recently a comment in the forum about the Swedish trait not giving a unit at Chivalry. It's really obvious why it doesn't. Here's the relevant SQL code from CEL_End.sql:

INSERT INTO Trait_FreeUnitAtTech
        (TraitType, TechType, UnitClassType, PromotionType)
SELECT  'TRAIT_ALLOTMENT', PrereqTech, Class, 'PROMOTION_ALLOTMENT'
FROM Units WHERE Type IN (
    'UNIT_ARCHER'               ,
    'UNIT_CATAPULT'             ,
    'UNIT_CHARIOT_ARCHER'       ,
    'UNIT_HORSEMAN'             ,
    'UNIT_SWORDSMAN'            ,
    'UNIT_SPEARMAN'             ,
    'UNIT_TRIPLANE'             ,
    'UNIT_GREAT_WAR_BOMBER'     ,
    'UNIT_ANTI_AIRCRAFT_GUN'    ,
    'UNIT_BIREME'               ,
    'UNIT_TRIREME'              ,
    'UNIT_SUBMARINE'            ,
    'UNIT_CARRIER'              ,
    'UNIT_LANCER'               ,
    'UNIT_HELICOPTER_GUNSHIP'
);

As you can see, the Knight unit is not included as part of the trait, In fact, many units are left out. More are left out actually than included. I encountered this a few days ago. In my mod, I updated this code to include every unit in the game for the Swedish trait. I actually ran across it because I was modding the trait for Carthage. I just found them to be too powerful, so I was altering their unique abilities. I ended up leaving their unique building and unique unit alone. I got rid of their free harbors in every city and free sailing tech. I made sure they start with a Bireme and a Trireme (so 2 ships) and they also get a free ship when a new naval tech is researched (like the Swedish trait, but navy only). I haven't test it yet. The free harbors was just too powerful. It gives them such a financial advantage having automatic city connections and very powerful trade routes from the beginning of the game. 9 times out of 10 Carthage was a powerhouse civ, so I'm looking for ways to nerf them.

Anyways, while altering the Carthage trait, I ran into that potential bug with the Swedish trait. It appears to be a bug, so in my own code I added every unit to that list. Now, I'm not really sure if it's a really "bug" or not, or if it was designed to include only some units. If it's really intended to include every unit, then the code is wrong. If it's supposed to include every unit, then I can fix that bug with a simple copy and paste since I've already done the work to find every unit in the game. Just let me know if it's really a bug or the mod was designed to include only some units for the Swedish trait.

GrantSP commented 10 years ago

As you can see from the code, the trait is to give units based on class. It looks like it wants to select classes using that list. Perhaps abit of tweaking is needed.

Regarding Carthage, have a look at the discussion in the forum I started.

stackpoint commented 10 years ago

It looks like the only two variables that our lua uses to spawn the allotment units is the TraitType, TechType, Class and PromotionType.

    local query = string.format("TraitType='%s' AND TechType='%s'", player:GetTraitInfo().Type, techInfo.Type)
    for row in GameInfo.Trait_FreeUnitAtTech(query) do
        local unitInfo = GameInfo.Units[player:GetUniqueUnitID(row.UnitClassType)]      
        local plot = centerPlot
        local unit = nil
        if unitInfo.Domain == "DOMAIN_SEA" then     
            plot = Plot_GetNearestOceanPlot(centerPlot, 10, 0.1 * Map.GetNumPlots())
            if not plot then
                log:Warn("Could not find large ocean near %s for free ship. Searching smaller bodies of water...", player:GetName())
                plot = Plot_GetNearestOceanPlot(centerPlot, 10)
            end
            if plot then
                log:Info("FreeUnitWithTech %s %s", player:GetName(), techInfo.Type)
                unit = player:InitUnitType(unitInfo.Type, plot)
            else
                log:Warn("No coastal plot near %s for free ship!", player:GetName())
                plot = centerPlot
                if plot:IsCoastalLand() then
                    unit = player:InitUnitType(unitInfo.Type, plot)
                end
            end
        else
            unit = player:InitUnitType(unitInfo.Type, plot)
        end
        local promotionType = row.PromotionType
        if promotionType then
            local promotionID = GameInfo.UnitPromotions[promotionType].ID
            unit:SetHasPromotion(promotionID, true)
        end
    end
GrantSP commented 10 years ago

This sql will, I think, give 1 of each type of unit that is military for each tech reached.

INSERT INTO Trait_FreeUnitAtTech
    (TraitType, TechType, UnitClassType, PromotionType)
SELECT  DISTINCT 'TRAIT_ALLOTMENT', PrereqTech, Class, 'PROMOTION_ALLOTMENT'
FROM Units WHERE Combat>0;

@stackpoint I was writing this just as you posted, so I could be overlapping your thought.

stackpoint commented 10 years ago

Looking at the initial sql code, Thal probably uses the Units table instead of the UnitClass table in order to access the PrereqTech easier. The SQL could probably be entirely automated by checking if the Unit has a production cost, if it has combat strength and if it is an unique unit so we don't get double UnitClass values.

stackpoint commented 10 years ago

Looks like you got ahead of me. Does the DISTINCT qualifier prevent duplicate unitclasses?

GrantSP commented 10 years ago

Using SQLite Spy I can populate this table with just 1 of each type of unit using that code.

stackpoint commented 10 years ago

Make sure to add the production cost check so they don't get free policy units like the foreign legion

GrantSP commented 10 years ago

There are 2 duplicates. 1 is the Mayan slinger, which is available at Agriculture and the other is the Legion available at Bronze Working as opposed to the other swordsmen at Iron Working. Is this an oversight I wonder?

stackpoint commented 10 years ago

It's because their PrereqTech is different from the standard units so DISTINCT must not catch it.

GrantSP commented 10 years ago

Yeah the DISTINCT is operational on the Class column. Maybe I can place 2 DISTINCT selection?

stackpoint commented 10 years ago

No, this should be a check done in the lua since we'd want the code to spawn units even for UUs with different PrereqTech values. Something like:

if unitInfo.PrereqTech ~= row.TechType then
     return
end
GrantSP commented 10 years ago

As the sql stands now the table is populated with 1 every type of unit, no planes or missiles. Do we want to include those also?

stackpoint commented 10 years ago

I'm not sure if the code would spawn them correctly... Is that with the production cost check?

GrantSP commented 10 years ago
INSERT INTO Trait_FreeUnitAtTech
    (TraitType, TechType, UnitClassType, PromotionType)
SELECT  DISTINCT 'TRAIT_ALLOTMENT', PrereqTech, Class, 'PROMOTION_ALLOTMENT'
FROM Units WHERE Combat>0 AND Cost>0;

This checks if they are military and are in the production queue.

stackpoint commented 10 years ago

One-shot units probably shouldn't be spawned but planes should alright. I just checked and the units are spawned on the capital so planes should be alright.

GrantSP commented 10 years ago

So am I correct in assuming the lua checks if the player has this trait, then what tech was reached and then gives them a unit that is unlocked at that tech.

stackpoint commented 10 years ago

Yes.

GrantSP commented 10 years ago

OK, revised selection:

INSERT INTO Trait_FreeUnitAtTech
    (TraitType, TechType, UnitClassType, PromotionType)
SELECT  DISTINCT 'TRAIT_ALLOTMENT', PrereqTech, Class, 'PROMOTION_ALLOTMENT'
FROM Units WHERE MilitaryProduction>0 AND Moves>1 AND Combat>0 OR Immobile=1;

This selects all military units that the player can build including missiles and aircraft. Still looking for a column to use that differentiates the missiles from aircraft.

stackpoint commented 10 years ago

What's the difference between MilitaryProduction and Cost?

(Combat>0 OR RangedCombat>0) AND Cost>0 AND Suicide=0
stackpoint commented 10 years ago

Hm, it seems like both the foreign legion and the landsknecht have production costs even though they can't be built.

GrantSP commented 10 years ago

Not a great deal. Some units have no Combat, like planes or missiles so they won't be picked with Combat>1. I chose MilitaryProduction since selects all of the desired units but we need the Cost column because we set some as unused by making their Cost -1. The Immobile column adds in the planes and missiles, since they are Combat=0 We still need Combat>0 because Workboats are classed as a military production for some reason.

I guess there could be other ways to select the needed units. That's just what I came up with as we are posting comments.

Just tried your selection, works just as well and is less clumsy.

GrantSP commented 10 years ago

The only reason the Landskneckt gets in the list is because it is it's own class type, the Foreign Legion are Great War Infantry and "should" be weeded out by the civilization check. Since they are not a UU for any civilization wouldn't the lua give the default GWI for your civ?

GrantSP commented 10 years ago

We can safely remove the Landskneckt with:

DELETE FROM Trait_FreeUnitAtTech
WHERE UnitClassType='UNITCLASS_LANDSKNECHT';
stackpoint commented 10 years ago
(Combat>0 OR RangedCombat>0) AND Cost>0 AND Suicide=0 AND PurchaseOnly=0

RangedCombat should get the planes while suicide removes the one-shot units and purchaseonly takes care of the Landskneckt.

GrantSP commented 10 years ago

Looks good.

stackpoint commented 10 years ago

Should be fixed here: https://github.com/Thalassicus/cep-bnw/commit/b2489239eb3c7affa47ca7e814901c4b83802b72