> > On Tue, 25 Dec 2012 11:02:52 AM Michael Vehrs wrote: > >>> [WOULDBENICE] Russian coat production >>> >> Fixed. The same problem had plagued the Swedish advantage. The solution >> was to limit the hammer production bonus to persons, so that it no >> longer applied to the carpenter's house. Unfortunately, the same fix had >> not been applied to the Russian advantage. >> > At least now we are consistent. However (as griped on the forums) the > Russians and Swedish *carpenters* now no longer appear to derive any benefit > from their national advantage. In the attached saved game, you can see that > after one turn, a Swedish master carpenter has and is producing 6 hammers, > which agrees with the production summary and building display, but not with > the "Show productivity modifiers" unit menu entry, which promises "Carpenter's > house" +3, "Master Carpenter" x2, "Building" +2 = 8, which I think is correct. > OTOH, the Swedish expert lumberjack is shown producing 10 lumber in all > displays, which is also consistent with the colony containing 10 - 6 = 4 > lumber after one turn, and does include the Building bonus. > > The menu entry calls "getProductionModifiers(goodsType, unitType)" for both > Buildings and ColonyTiles, which is blind to colony conditions, yielding ideal > values. The production panel is calling Colony.getAdjustedNetProductionOf > which works off the production cache and so may be expected to vary from the > ideal, however I do not see why it should in this case. Since Building > differs > from ColonyTile, looking into Building, the key routine in > getAdjustedProductionInfo, and the only place it takes account of unit > production is by looping over the units present and accumulating the result of > getUnitConsumption(). getUnitConsumption is not picking up the national > advantage modifier, so thars yer problem. > > Fixing it however is not easy. The obvious fix is to just replace the logic > in > getUnitConsumption() with a call to getPotentialProduction() (patch1 > attached), which relies on getProductionModifiers() and thus yields consistent > results. This does in fact fix the Swedish carpenter. > > However it also causes several test fails, with bells being problematic. > The trouble is that the Town Hall is specified as follows: > > <building-type id="model.building.townHall" basicProduction="3" > produces="model.goods.bells"> > <modifier id="model.goods.bells" type="additive" value="1"/> > </building-type> > > and the test suite is enforcing the requirement that the additive modifier be > applied once, and once only, independent of the number of units present in the > building, including 0. So as it stands, getProductionModifiers() just gets > all > applicable modifiers, and thus if we apply its resulting modifier set across > all > units the magic once-only addition gets picked up per unit resulting in > overproduction. > > OK. So at this point I read the comment on getUnitConsumption that says that > the routine should return only the production specific to the unit, not that > specific to the building. This is a poorly specified concept, as you might > think that the building basic production was specific to the building, but it > clearly does apply on a per unit basis,
I think the basic production should be considered to apply per work-place rather than per unit. I think it would be possible to specify basic production per unit type, and add a multiplicative modifier to higher-level buildings, but I am by no means sure that would be any clearer. > so eventually I ended up with: > > final BuildingType type = getType(); > final Turn turn = getGame().getTurn(); > // Get all the production due to the building. > int buildingSpecific = (int)FeatureContainer.applyModifierSet(0f, > turn, type.getModifierSet(output.getId(), type, turn)); > int potential = getPotentialProduction(getGoodsOutputType(), > unit.getType()); > return potential - buildingSpecific; > > This does filter out that additive bonus. However it is getting ugly and > unclear, and still throws test failures as it does not correctly handle the > effect of other production modifiers such as Jefferson and Printing Press. > > That is far as I can get right now. I am beginning to think that we need some > spec-level differentiation between building modifiers that apply independent > of > units (i.e. the bell and cross bonuses that work even for empty buildings) > and the per-unit modifiers (all the others AFAICT). I agree. > A real bonus would be if > we can get to a final state where all work locations could have their peak > productivity evaluated with something that calls > getProductionModifiers(goodsType, unitType) for each unit, and then > getProductionModifiers(goodsType, null) to pick up non-unit production. > > Cheers, > Mike Pope > I don't see any elegant solution at the moment, but I'll think about the problem. Regards Michael ------------------------------------------------------------------------------ Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and much more. Get web development skills now with LearnDevNow - 350+ hours of step-by-step video tutorials by Microsoft MVPs and experts. SALE $99.99 this month only -- learn more at: http://p.sf.net/sfu/learnmore_122812 _______________________________________________ Freecol-developers mailing list Freecol-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freecol-developers
