[API Script] ADD param for get systemtype

[Yona-TYT]

I made a small patch to include the systemtype parameter which I have also needed for a long time.
0001-ADD-param-SystemType.patch


I have a question regarding "systemtype_t":

Code Select Expand

/**
 * System types for ways
 */
enum systemtype_t {
type_flat     = 0,   ///< flat track
type_elevated = 1,   ///< flag for elevated ways
type_runway   = 1,   ///< flag for runway (only aircrafts)
type_tram     = 7,   ///< tram track (waytype = track_wt)
type_river    = 255, ///< flag for river
type_all      = 255  ///< special ?
};Why is there no {type_invalid = -1} like waytype does?, and why is the 255 duplicated?.

[Andarix]

for walls, fences ...

Code Select Expand

waytype = track
system_type = 255

for river

Code Select Expand

waytype = water
system_type = 255


[Andarix]

I think 64 is missing

Wiki - way def - system_type

[prissi]

Duplicated types are just for masks. systemtype is unsigned 8 bit in the file, so -1=255 Fences could be any number, only rivers, trams and runway and elevated have fixed function.

[Yona-TYT]

trams and runway and elevated have fixed function.

Elevated ways is the only thing I need for the tutorial, in the tests I did it worked very well.

[Yona-TYT]

I made a small patch to include the systemtype parameter which I have also needed for a long time.
0001-ADD-param-SystemType.patch

Dear @Prissi , is there something wrong with the patch? I would like to know if this can be implemented.

Regards.

[prissi]

The system type is tied to the default parameter, which can be checked already on init. Checking later seems wrong, because you should not be allowed to select a tool and then get later an error when using it.

Thus this is hackish as most dragging tools never have a system type anyway and st is a cryptic variable name.

Why do you not check the system type on selection and wait instead for the first click?

[Yona-TYT]

The system type is tied to the default parameter, which can be checked already on init. Checking later seems wrong, because you should not be allowed to select a tool and then get later an error when using it.

I don't understand, are you saying that the system_type can be obtained using a different method?

Thus this is hackish as most dragging tools never have a system type anyway

In that case, can you condition it so that get_systemtype() is called only if it is a tool to build way?.

and st is a cryptic variable name.

Ok, I'll change the name to: system_type

Why do you not check the system type on selection and wait instead for the first click?

I don't understand you well, can you give me an example?.

[prissi]

Is tool allowed show never take the waytype as third parameter. It should get the default_parameter for that tool, as the effect of a tools strongly depends on that. With the default parameter, you can lookup a way for the way building tool or only allow to build passenger stations.

That hack started even earlier when passing a waytype to is_tool_allowed. Many (most) tools have no waytype. Adding this was already a hack.

The proper way would be in void tool_selector_t::add_tool_selector(tool_t *tool_in) calling
welt->get_scenario()->is_tool_allowed(welt->get_active_player(),tool); (and then in the few other places that uses this function.)

Then in is_tool_allowed it will call the script with the toolnumber and the default string. The script would then need to find out if it is a way to be build, a bridge, a station, ... and then lookup the object using the default parameter string and finally decides to allow the tool or not based on the object it finds.

[prissi]

The waytype could be still used for more general forbiddings, like either a waytype or waytype==invalid and then the default_parameter could be tested on the script side. But on the simutrans side, the proper use would be to also transfer the default_paramter. Makes the list of forbidden tools larger, but then it should work for whatever specific requests comes next.

[Yona-TYT]

calling
welt->get_scenario()->is_tool_allowed(welt->get_active_player(),tool);

I think you are confusing "is_tool_allowed" (for the tool menus) and "is_work_allowed_here" which is when you click on the map to build or delete something.

Edit.

The waytype could be still used for more general forbiddings, like either a waytype or waytype==invalid and then the default_parameter could be tested on the script side. But on the simutrans side, the proper use would be to also transfer the default_paramter. Makes the list of forbidden tools larger, but then it should work for whatever specific requests comes next.

It seems reasonable to me, although I don't really understand how this works so it will take me a while to get it.

[prissi]

They both would need the same mechanism. It does not make sense to have two different ways. But the proper way would be using the default param. Maybe both, system type for general disallowments and the default parameter for a closer check.

[Yona-TYT]

They both would need the same mechanism. It does not make sense to have two different ways. But the proper way would be using the default param. Maybe both, system type for general disallowments and the default parameter for a closer check.

Yes, I already imagined that, the same mechanism to solve everything.

[Yona-TYT]

Sorry Isaac for the double post.

Ok, I did the following experiment and it seems to work fine, so should I do the same with waytype?.

0001-ADD-param-SystemType-TEST.patch

[Isaac Eiland-Hall]

Sorry for the spam, but in recent years, the double-post rule was modified with a rule of thumb: If your post is taller than the profile stuff on the left, it's okay. That's what the rule was meant to combat: Many short posts that wasted vertical space.

To me, this is fine: https://i.imgur.com/jvgdq0L.png

[prissi]

I think you did not understand my message. The system is broken, as there is no way to filter for the parameter string, which determines also the system type of a way. Adding parameters to tools that do not need them is curing symptoms. I will do an overhaul, that adds support for a defualt_parameter string to the script interface, so you can exclude tools which use certain ways but allow others.

That makes much more sense for real scenarios anyway, like allow only dirt roads for street building as last mile transport but allow all kind of rails. That is not possible with the current system, as there is no way to find out if the dirt road is build or not.

[Yona-TYT]

I see, this is going to require deeper changes than I'm used to.

It's interesting because by using a single mechanism for everything, I have to assume that the same tools filtered for the menus will also be used in the other places where they are required for the scripts. 

[prissi]

Here is a patch. Since it adds another paramter to is_tool_allowed and is_work_allowed, none of the old scripts will work. The needed to have one paramter more.

[Yona-TYT]

Here is a patch. Since it adds another paramter to is_tool_allowed and is_work_allowed, none of the old scripts will work. The needed to have one paramter more.

That's no problem, there aren't many scripts that use this and we can update all of them.

Edit.

In script_compat.nut something can be done to manage compatibility I think

Edit:
This is great, now I can get del way desc easily, thanks a lot!. 

Code Select Expand

local way_desc =  way_desc_x.get_available_ways(wt_road, st_flat)
foreach(desc in way_desc){
if(desc.get_name() == parm){
gui.add_message("Curr name: "+ parm+" SystemType: "+desc.get_system_type())
}
}

[Yona-TYT]

Add the parameter to scenario_base.nut.

I will take care of making the necessary adjustments in the scripts myself, it is not that difficult actually and in our small community there are not many scripts/scenarios.

I also think that the automated tests need to be adjusted.

0001-Add-new-param-in-scenario_base.nut.patch


I made a temporary fix for testing, I don't have much time now to make the new parameter part of testing.
0001-Fix-Test.patch

[prissi]

In principle, by using "" or 0, one should get the old behaviour. This is only needed if one want to allow only certain ways.

[Yona-TYT]

There is a return false in "const char* scenario_t::is_work_allowed_here", I'm not sure if this is a break or a return with blank message "".