News:

Simutrans Wiki Manual
The official on-line manual for Simutrans. Read and contribute.

defining explicit object in menuconfig without explicit image not possible?

Started by Leartin, July 03, 2014, 10:39:12 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Leartin

This might be intended or just not used, but it strikes me as strange, so I report it.

In the menuconfig it is possible to define the position of a single object, eg. like this:
toolbar[1][1]=general_tool[14],4,,dirt_road
this works, and I am able to place a dirt_road with that configuration. However, even though the dirt_road has it's own icon, the following does not work:
toolbar[1][1]=general_tool[14],,,dirt_road
The dirt_road is available at every time. When using another way, the icon was visible but the way not yet available, so you couldn't click on the icon. It worked fine once the way was available.

The desired effect would be for the toolbar to show the icon that comes with the object, only as long as the object is available, or show nothing at all if the named object could not be found.

prissi

It only shows something different, if the icon is assigned to that tool explicitely. Otherwise it should not have an icon, if it is not available. I have to check for an error indeed.

Leartin

May I bump this? (I think it's forgotten...)

It's still as stated before
- either I do assign an icon in the menuconf, and it's always on the menu, but not clickable if the way is not available at the time
- or I do not assign an icon in the menuconf, and it's never on the menu, even if the time is right.

I'd think the intended behaviour is to show the icon only if the way is available, regardless whether an icon is assigned in the menuconf or not.

prissi

The waybuilder does not know which way to build when the menu icon is generated as it does not know its argument yet (the icon is argument independent!). As such it cannot show a "correct" image. There is an internal tool generated by the waybuilder though.

Dwachs

Why not modify tool_build_way_t::get_icon to check for availability of the way object?
Parsley, sage, rosemary, and maggikraut.


Leartin

Prissi, thank you for taking the time to do that. Sadly, it only applies for ways.
While I used a way as an example in this thread, it really was just an example, and I hoped there would be a solution for all kinds of objects (bridges, tunnels, stations, signals, wayobjects, possibly even depot and extension buildings for consistency, even though these probably don't need it)

Looking at the code you used, wouldn't it be possible to remove the parts about trams and underground mode (except for stations) and apply it to the other kinds of objects? At least stations check for underground mode already anyway, so a get_icon function is already called, wouldn't it be sufficient to replace

if (besch == NULL) {
return IMG_LEER;
}

with the relevant parts of the other code:
image_id bild = icon;
if(  besch  ) {
if(  bild ==  IMG_LEER  ) {
bild = besch->get_cursor()->get_bild_nr(1);
}
if(  !besch->is_available( world()->get_timeline_year_month() )  ) {
return IMG_LEER;
}
}

(If I read it correctly, the first inner if gets the icon from the object so it can be displayed, while the second returns an empty image if the object is not available at the time.
If so, wouldn't it be marginally faster to swap position, so it would not even get the image if it isn't going to be displayed?)

-Sorry, I cannot program and it might be completely wrong.

prissi

Compared to the time actually drawing anything it will not matter.

And for any other object the code will have to be copied, as you noticed. I am travelling on MOnday, so I am not able to do this, but gladly accept patches on that.

Leartin

So this was one of the things I wanted to try for myself after I get a working compiling setup, since it seems like something doable via trial and error. But since I could not even get anything to run, would someone be so kind to do this for me please?

In case someone wonders what that is good for:
A) You could add more buildable elements to the base pakset if they would not clutter the menu. With this, we can choose precisely which elements, eg. which extension buildings or which signals are in the main railway menu, while the 'normal' way to define those elements in bunches would be put in purposefully cluttered sub-menus for advances players. One would only have one signal per type in the main menu, but via a sub menu you could still choose among various graphical representations for the same functionality. Therefore, those who seek functionality and those who seek beauty don't need to fight over what should be in a pakset.
B) Putting things that belong together together, even if they are different objects. One could combine a way, elevated way, tunnel portal and bridge with the same speed and design and have them together in the menu. Or even different ways that belong together stylistically, eg. all the harbour-ways in p96c.

userfriendly

I'm working on a possible resolution to this bug and would like some input, if I'm on the right track. Here's the test code: https://github.com/user-friendly/simutrans/commit/362b5c5740c6171a2dac362d4d8fa11ce7ff4af4 Would really appreciate some input (code style is temporary).

Currently works only for roadsigns (should be of any way type). I'm new to Simutrans' codebase, but looking at the code in simmenu and simtools, I think it might be more appropriate to setup these "custom" general tools at load time and add a check in toolbar_t::update() for tool availability (tools that have timeline description objects), right after the scenario check.
Just be good to each other.

userfriendly

Sorry for the double post. Attached is, perhaps, a better version of the previously proposed fix.
Just be good to each other.

Dwachs

Looks good, did not test :)

I think the switch blocks can be simplified by casting all the desc-pointers to their common base class obj_desc_transport_infrastructure_t, i.e.,

if (tool->get_default_param()) {
const obj_desc_transport_infrastructure_t* desc = NULL;
switch (id) {
case TOOL_BUILD_WAY:
{
const way_desc_t *desc = way_builder_t::get_desc(tool->get_default_param());
break;
}
[..]
}
if (desc && desc->get_builder() && desc->get_builder()->get_id() == tool->get_id()) {
return desc->is_available(time);
}

Their could be even a helper function that returns the desc-pointer for a given tool and parameter string.
Parsley, sage, rosemary, and maggikraut.

userfriendly

Comments addressed. As for a helper function, I couldn't find one, but then again, I've been mostly looking at simmenu.h/.cc.
Just be good to each other.

Dwachs

Modified the patch to reduce code duplication. It should still work. Please let me know of any errors. imho this can be submitted. Thanks for your work!
Parsley, sage, rosemary, and maggikraut.

userfriendly

Code compiles without errors. Ran a quick play test - no runtime issues.

Just realized the second paragraph of the doc block for
set_defaults_general_tool()
is useless, so I think it can be removed, but obviously, not a blocker.

Thanks for taking the time to review the proposed patch and the feed back.
Just be good to each other.

Dwachs

Parsley, sage, rosemary, and maggikraut.