Removal of "hardcoded" DLL and Python logic [IMPLEMENTED]

[raystuttgart]

WTP still has a few places that have "hardcoding" or "abuse of unrelated XML tags" in its logic.
Thus let us collect them all and try to remove them step by step ...

-----

Why bother to remove these things?
Isn't it working currently?

Well, yes it is working currently and code improvements as such do not add new features.
But it will make the mod much more stable and easier to maintain in the long run.

If a mod-modder not knowing that code should ever change XML it can easily break the intended DLL logic.
It is of course also pretty bad sytle of coding ...

But well there were times when I was a beginner as well.
My main focus was simply to get all that stuff working with little effort - not if it was "good style".

-----

This here e.g. is an example that has both.

It is one of the oldest pieces of logic I had ever written.
(It was even before RaR - it was still in RWL ("Ray's Wish List") which later became RaR to a huge degree.)

Hardcoding example: getSpecialBuildingType() == 26
Abuse of unrelated XML tags example: GC.getBuildingInfo(eBuilding).getAICitySize() == 4

Here's another suggestion for removal of hard-coding: Add a (Free)Route attribute (or similar) to BuildingInfo:

"CvCity:: ProcessBuilding"

//RWL Railroads and Trainstations
if (GC.getBuildingInfo(eBuilding).getSpecialBuildingType() == 26) // easiest way to identify Trainstation
{
//WTP fixing small bugs in City Founding an Roads
CvPlot* pPlot = plot();
if (pPlot->getRouteType() < 2)
{
pPlot->setRouteType((RouteTypes)2);
}
}
//RWL END Railroads and Trainstations
// R&R, ray, Plastered Road
if (GC.getBuildingInfo(eBuilding).getAICitySize() == 4) // easiest way to identify Townhall
{
//WTP fixing small bugs in City Founding an Roads
CvPlot* pPlot = plot();
if (pPlot->getRouteType() < 1)
{
pPlot->setRouteType((RouteTypes)1);
}
}
// R&R, ray, Plastered Road -END

-----------

@devolution:
I will take care of the example you posted.

Buildings will get this XML attribute:

RouteTypeCreatedOnCityCenterPlot
(it will be able to contain "RoutTypes" and will be optional in XML)

 

[Nightinggale]

While we are talking hardcoding, I will point out a change I made. We should rely more on enum values.

for (RouteTypes eRoute = FIRST_ROUTE; eRoute < NUM_ROUTE_TYPES; ++eRoute)

This is how for loops should be written. There is a FIRST and NUM for each enum. We used not to do this because it would hardcode the values and the ++ operator doesn't exist in vanilla. I changed NUM_ROUTE_TYPES to be a global variable, which is set just after reading the xml files and it can be used instead of GC.getNumRouteTypes. In fact in general NUM_ROUTE_TYPES should be used instead of asking GC and this goes for all enum types.

The benefits of doing this is in addition to less annoying calls while debugging, it will present the data as enums while debugging, like ROUTE_ROAD(0) instead of 0. Also since it's an enum, strict types can help catch bugs (etc swapped function arguments). It's also faster. If we decide to optionally hardcode data in the DLL while compiling, NUM_ROUTE_TYPES will become hardcoded. This means the compiler will know more while compiling like how many times a loop will iterate and it can use that for better optimization, resulting in faster runtime.

Another benefit is that we can keep variables as enum instances and that way avoid typecasts. We should avoid typecasts as much as possible because that's something, which is good at hiding bugs.

Hardcoded yields
Also regarding getting rid of hardcoding, I created a branch where I added yield categories. The idea is that the categories are hardcoded (has to if we want to use switch case), but the categories are generic and controls some basic stuff like "raw material", "manufactored goods", "weapons" etc. This in turn allows the yields themselves to not be hardcoded and instead apply to a category and (I think) 6 bools for behavior, which includes how AI views the yields. With this in place, more yields can be added without even recompiling the DLL.

It's done. It just needs to be updated and then merged.

 

[raystuttgart]

It's done. It just needs to be updated and then merged.

Sounds great.

But this here is not fixed yet right?
(Just so I do not waste my efforts.)

Here's another suggestion for removal of hard-coding: Add a (Free)Route attribute (or similar) to BuildingInfo:

"CvCity:: ProcessBuilding"

//RWL Railroads and Trainstations
if (GC.getBuildingInfo(eBuilding).getSpecialBuildingType() == 26) // easiest way to identify Trainstation
{
//WTP fixing small bugs in City Founding an Roads
CvPlot* pPlot = plot();
if (pPlot->getRouteType() < 2)
{
pPlot->setRouteType((RouteTypes)2);
}
}
//RWL END Railroads and Trainstations
// R&R, ray, Plastered Road
if (GC.getBuildingInfo(eBuilding).getAICitySize() == 4) // easiest way to identify Townhall
{
//WTP fixing small bugs in City Founding an Roads
CvPlot* pPlot = plot();
if (pPlot->getRouteType() < 1)
{
pPlot->setRouteType((RouteTypes)1);
}
}
// R&R, ray, Plastered Road -END

 

[Nightinggale]

But this here is not fixed yet right?
(Just so I do not waste my efforts.)

I don't think anybody fixed that if it's not fixed in develop/plains. It's certainly not something I touched while working on yields.

 

[raystuttgart]

@devolution
This is now taken care of.
Here is the commit.

@community:
The new XML-tag in CvBuidlingInfos.xml is called:
RouteTypeCreated