Thalassicus / cep-bnw

Civ V Communitas Expansion Pack
32 stars 22 forks source link

Churches of Lalibela bug missionary extra spread #133

Closed Hawawaa closed 10 years ago

Hawawaa commented 10 years ago

It gives you the extra missionary but the missionary only gets 2 spreads not 3.

stackpoint commented 10 years ago

It looks like the missionary is generated before the +1 spread is set. Not sure how to fix this.

Either have to create custom lua code to generate unit spawns or it may be possible that a missionary created by a dummy building created by Lalibela would be enough abstraction for the +1 spread to take effect.

GrantSP commented 10 years ago

Another way would be to make a unique missionary that is granted as free from this wonder. Give it 3 ReligionSpreads. Make it so this new unit is not available in the Faith Purchase queue and the new missionaries should, by that time, have the extra spread applied to them.

Six of one, half a dozen of the other as to which is easier. Dummy building or dummy unit.

stackpoint commented 10 years ago

This sounds like a good plan. The only problem may be making the missionary invisible.

GrantSP commented 10 years ago

I was thinking along the lines of that new non-religious Conquistador that @Thalassicus devised

stackpoint commented 10 years ago

Sounds like a plan.

GrantSP commented 10 years ago

This is really strange. I've tried 2 methods to get this to work.

Both ways give a unit with only 2 spreads, despite the new unit showing as the free unit. If I grant a free unit with fire tuner the new unit has 3 spreads and an additional 1 if given after the wonder is built, so the wonder itself is working fine.

It just doesn't make any sense! Will keep trying.

stackpoint commented 10 years ago

I presume you're granting the unique missionary instead of the vanilla one?

        <Row>
            <BuildingType>BUILDING_CHURCHES_LALIBELA</BuildingType>
            <UnitType>UNIT_MISSIONARY</UnitType>
            <NumUnits>1</NumUnits>
        </Row>
GrantSP commented 10 years ago

I just now renamed the new unit to show that it was indeed the new one that is granted!

Now it gets even weirder, despite no missionary units being granted as free by ANY building except my new one by the free building (database only shows UNIT_MISSIONARY_LALIBELA as coming from the dummy building BUILDING_LALIBELA_MISSIONARY_SCHOOL) the unit that is given when this building is built is a VANILLA with 2 spreads!

stackpoint commented 10 years ago

Can you post the code you're using to generate the unique missionary?

Hawawaa commented 10 years ago

Another option would be the wonder gives you 2 normal missionaries(extra spread but if this works it works) or 1 normal missionary and 1 missionary with 1 spread.

GrantSP commented 10 years ago

civ5screen0005 Code for the unit is fine. This image shows the new unit with the correct specs and with the tooltip "Chinese Missionary with ExtraMissionarySpreads" just to the north of Beijing. I gifted this with FireTuner.

In the city the new missionary is the one given by the new wonder/dummy building. As you can see it is just a vanilla missionary.

Building_FreeUnits has the new unit set and it correctly shows in the database.

GrantSP commented 10 years ago

I've made a new branch called Testing and committed this code for your perusal. It is [here].(https://github.com/Thalassicus/cep-bnw/commit/3851f24ce5d873f0ef58c77bccc335caa7ecc0b5)

stackpoint commented 10 years ago

Building_FreeUnits is vanilla code. And your code looks fine.

GrantSP commented 10 years ago

Yeah I know it is vanilla. I just had a quick look at the source to see if perhaps there is something hard-coded related to that function.

    kUtility.PopulateArrayByValue(m_piNumFreeUnits, "Units", "Building_FreeUnits", "UnitType", "BuildingType", szBuildingType, "NumUnits");

That is all the stuff I can find on the tag and it looks like it just uses the data in the table, as it should.

stackpoint commented 10 years ago

It looks like it take the UNITCLASS value from the UNIT and grants the unit that way. Which is why it defaults to the 2 spread missionary. You can try creating a new UNITCLASS to go with the new UNIT.

                    for(iFreeUnitLoop = 0; iFreeUnitLoop < pBuildingInfo->GetNumFreeUnits(iUnitLoop); iFreeUnitLoop++)
                    {
                        // Get the right unit of this class for this civ
                        const UnitTypes eFreeUnitType = (UnitTypes)thisCiv.getCivilizationUnits((UnitClassTypes)pkUnitInfo->GetUnitClassType());

                        // Great prophet?
                        if(GC.GetGameUnits()->GetEntry(eFreeUnitType)->IsFoundReligion())
                        {
                            GetCityCitizens()->DoSpawnGreatPerson(eFreeUnitType, true /*bIncrementCount*/, true);
                        }
                        else
                        {
                            pFreeUnit = owningPlayer.initUnit(eFreeUnitType, getX(), getY());

                            // Bump up the count
                            if(pFreeUnit->IsGreatGeneral())
                            {
                                owningPlayer.incrementGreatGeneralsCreated();
                                if (!pFreeUnit->jumpToNearestValidPlot())
                                    pFreeUnit->kill(false); // Could not find a valid spot!
                            }
                            else if(pFreeUnit->IsGreatAdmiral())
                            {
                                owningPlayer.incrementGreatAdmiralsCreated();
                                CvPlot *pSpawnPlot = owningPlayer.GetGreatAdmiralSpawnPlot(pFreeUnit);
                                if (pFreeUnit->plot() != pSpawnPlot)
                                {
                                    pFreeUnit->setXY(pSpawnPlot->getX(), pSpawnPlot->getY());
                                }
                            }
                            else if (pkUnitInfo->GetUnitClassType() == GC.getInfoTypeForString("UNITCLASS_WRITER"))
                            {
                                owningPlayer.incrementGreatWritersCreated();
                                if (!pFreeUnit->jumpToNearestValidPlot())
                                    pFreeUnit->kill(false); // Could not find a valid spot!
                            }                           
                            else if (pkUnitInfo->GetUnitClassType() == GC.getInfoTypeForString("UNITCLASS_ARTIST"))
                            {
                                owningPlayer.incrementGreatArtistsCreated();
                                if (!pFreeUnit->jumpToNearestValidPlot())
                                    pFreeUnit->kill(false); // Could not find a valid spot!
                            }                           
                            else if (pkUnitInfo->GetUnitClassType() == GC.getInfoTypeForString("UNITCLASS_MUSICIAN"))
                            {
                                owningPlayer.incrementGreatMusiciansCreated();
                                if (!pFreeUnit->jumpToNearestValidPlot())
                                    pFreeUnit->kill(false); // Could not find a valid spot!
                            }                           
                            else if (pFreeUnit->IsGreatPerson())
                            {
                                owningPlayer.incrementGreatPeopleCreated();
                                if (!pFreeUnit->jumpToNearestValidPlot())
                                    pFreeUnit->kill(false); // Could not find a valid spot!
                            }
                        }
                    }
GrantSP commented 10 years ago

Well I guess that makes sense. Sort of. Don't want to give unique units of one civ to the oppostion. I wonder why they set up the Building_FreeUnits \ to use UnitType instead UnitClassType the way Policy_FreeUnitClasses** does?

GrantSP commented 10 years ago

Hurray!!!

We have success. All thanks to @stackpoint correctly finding the stupid vanilla code that Firaxis uses.

Will tidy up the code and post immediately.