News:

Want to praise Simutrans?
Your feedback is important for us ;D.

Schedule features: technical discussion

Started by jamespetts, January 22, 2018, 11:15:36 PM

Previous topic - Next topic

0 Members and 2 Guests are viewing this topic.

Ranran

#175
QuoteThe "modify_convoy" button does not work, but I infer that this is because this interface has not been designed yet - I presume that this is for consist orders? I cannot comment on the completeness of what you have implemented here without seeing the implementation of the modify_convoy dialogue.
QuoteAs to the consist orders feature, I think that Ves had originally planned to work on this, too, but it was separate from the "vehicle manager" feature. The consist orders feature has been discussed extensively in this thread, although I cannot recall how much discussion specific to the UI that there has been.
I think the vehicle manager is an independent entity.

https://forum.simutrans.com/index.php/topic,18113.msg184276.html#msg184276
And as far as I can tell from this thread, there's little mention of the UI, and Ves seems to have been working on something, but I couldn't find such code in his repository.
Therefore, if we want to implement this, we have to start the implementation of this UI from a blank slate. I think this will be a tremendous task.
First of all, as ves hinted at, there is currently no optimal vehicle pick component to achieve this.

Secondly, what we may have overlooked is that the consistent order must take into account the reversing state of the vehicle. vehicle_desc_t alone does not have a field to set the reverse state.
I'm not sure, but since OPRP is based on standard, I suspect that convoys with opposite orientations will not bind to each other. But with extended, that happens.
For example, when you open the replace frame of a convoy that is reverse-cruising in the replace frame of the current master branch, the connections you see are completely corrupted.
However, regarding this problem, I think that I fixed this in the overhaul branch of the replace dialog by the method via vehicle_t. This is just to change the order back and always change the same order when the convoy is reversed.
In the recombination system, however, we have to go one step further and maintain, understand and manage the reversing state.
It should be noted that if the vehicle is reversed, the coupling constraint will also be reversed.
Double-headed electric locomotives may not care about orientation. In other words, it may be oriented freely.
Players have to know that and set up the configuration, and the system and UI have to deal with it, which is feared to be very complicated.

Thirdly, as previously discussed with ACarlotti, coupling constraint originally had to consider the reversal state, but now it does not consider which side to connect with.
For example, the rear of the locomotive requires a tender, but if the tender is reversed, it will be judged to be in a good connection.

Fourthly, if the recombination system causes the convoy's configuration to change independently of the convoy's reverse state, the reversing process must also be revisited.
That is, currently the vehicle can be in the reverse state only when the convoy is in the revese state. This condition no longer holds.
For example, when a convoy that is not in the reverse state of all vehicles is moving forward, a convoy in the opposite direction may be added at the station. As such, convoy is not in the reverse state, but there is a possibility that vehicles in the reverse state will be mixed. Therefore, it is necessary to review the process of changing direction.

jamespetts

Thank you for that analysis, both as to the issue with respect to saving the schedule flags and also the reversing. I will have to look into those more closely in due course; the schedule saving issue once I have finished inflation and the reversing issue once I start to implement the convoy re-combination features in substance.

However, I think that it will be necessary to have the consist order UI (not the more sophisticated vehicle manager UI that Ves was planning) working at least in a basic state before I can start work on the code for convoy re-combination.

Can I check - is the 2207-overhaul-of-replace-dialog intended to be merged into the 15.x branch?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

#177
Quote from: jamespetts on July 10, 2022, 02:36:09 PMCan I check - is the 2207-overhaul-of-replace-dialog intended to be merged into the 15.x branch?
It is intended to be merged in the master and 15.0. But note that 2207-overhaul-of-replace-dialog is an old branch.
The latest branch is 2207-overhaul-of-depot-dialog-v3.
It's a large update of the basic and important dialogs with lots of changes, and I think it needs to be thoroughly tested, so I think it needs to be thoroughly tested on the master branch.
Also, without this change, updating the depot/replace dialog for 15.0 (adding many parameters) would be difficult.
The current old style depot dialog is a bit broken at this point as some ppl reported. I'm not sure if this is a conflict with the old code and it's difficult to fix.
Anyway, the update of depot dialog and replace dialog is almost completed, so basic test is possible.

EDIT:
Unfortunately, there seems to be a processing error somewhere in depot code.

jamespetts

Quote from: Ranran on July 06, 2022, 02:22:55 PMI suspect that the current code is not recognizing as intended after discharge_payload(65536).


bool is_flag_set(schedule_entry_flag flag) const { return flag & flags; }I think the related behavior was broken because the expression was wrong.


I have spent some time looking into this. I have tested specifically the is_flag_set() code, and this appears to work. Have a look at my testing code at line 1491 of schedule_gui.cc. Using lay_over as the test datum, setting lay_over here works correctly and TEST_set returns true. The flags value of the relevant schedule entry is correctly set to 2.

However, when the schedule window is closed and re-opened again, the value becomes corrupted. Instead of being 2, it is 1 (which is the flag for wait for time). Setting lay_over again correctly sets the value to 3, and, again, TEST_set evaluates to true. Closing and opening the schedule window, once again the flags value is reset to 1. It seems that something is overwriting the flags value when the schedule window is closed and/or opened. This is not code that I have written, so I am not sure where to look to find where this is happening.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

Quote from: jamespetts on July 10, 2022, 11:54:01 PMI have spent some time looking into this. I have tested specifically the is_flag_set() code, and this appears to work.
I fixed it last week. The #174 post was intended to report that. It is possible that the correct English could not be written.

Ranran

Quote from: jamespetts on July 10, 2022, 11:54:01 PMUsing lay_over as the test datum, setting lay_over here works correctly and TEST_set returns true. The flags value of the relevant schedule entry is correctly set to 2.

However, when the schedule window is closed and re-opened again, the value becomes corrupted. Instead of being 2, it is 1 (which is the flag for wait for time). Setting lay_over again correctly sets the value to 3, and, again, TEST_set evaluates to true. Closing and opening the schedule window, once again the flags value is reset to 1. It seems that something is overwriting the flags value when the schedule window is closed and/or opened. This is not code that I have written, so I am not sure where to look to find where this is happening.
And I believe now I have fixed it.

Ranran

Quote from: Ranran on July 09, 2022, 08:43:53 AMwe have to start the implementation of this UI from a blank slate. I think this will be a tremendous task.
Quote from: jamespetts on July 10, 2022, 02:36:09 PMHowever, I think that it will be necessary to have the consist order UI (not the more sophisticated vehicle manager UI that Ves was planning) working at least in a basic state before I can start work on the code for convoy re-combination.
The first thing I got stuck in was that I had no idea how to add, edit or remove constist_order_t.

jamespetts

Excellent, thank you for that - I will test this fix when I get a chance.

Quote from: Ranran on July 11, 2022, 04:08:52 PMThe first thing I got stuck in was that I had no idea how to add, edit or remove constist_order_t.
Do you mean how to edit the code itself or how to edit the data in consist_order_t? If the latter, I may not have added getters and setters yet; that will need to be my next job if this is so that you can get on with this. I had written the consist_order_t code so long ago that I have forgotten quite how far that I had got with it: my apologies. Do clarify which of the two meanings that you intend so that I know whether I need to work on getters and setters for consist_order_t. Thank you.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

Quote from: jamespetts on July 11, 2022, 04:12:22 PMDo you mean how to edit the code itself or how to edit the data in consist_order_t? If the latter, I may not have added getters and setters yet; that will need to be my next job if this is so that you can get on with this. I had written the consist_order_t code so long ago that I have forgotten quite how far that I had got with it: my apologies. Do clarify which of the two meanings that you intend so that I know whether I need to work on getters and setters for consist_order_t. Thank you.
The latter.
It means adding a new empty constist_order_t (and editing or deleting it) to the schedule entry. I think it's the same as adding a new line/convoy to the world. I don't see anything like the definition of tool.

jamespetts

Quote from: Ranran on July 11, 2022, 04:17:43 PMThe latter.
It means adding a new empty constist_order_t (and editing or deleting it) to the schedule entry. I think it's the same as adding a new line/convoy to the world. I don't see anything like the definition of tool.
Thank you for clarifying - that is helpful. I will look into this when I next get a chance to work on the code.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

Incorporating the update of the master branch seems to cause problems in obtaining vehicle data regarding inflation and prevents the game from launching.

PJMack

About seven years ago, I downloaded and rewrote a patch for simutrans standard where full vehicles would skip over destinations with nothing to unload and empty vehicles would skip over destinations where there was no potential for picking up goods (set by setting the minimum loading to not zero, a hack but I did not understand the GUI code at the time).  This resulted in being able to create distribution centers where a train would drop goods off at each distribution hub where trucks waiting at that hub would load.  The trucks would then proceed directly to the destinations of the loaded goods to drop them off, then return directly to the hub to pick up or wait for more goods.  Unfortunately, my record keeping abilities from that time were rather lacking and have been unable to find and properly credit the makers of the original patch, however I put some of the code below that I believe I wrote most of but may contain a few lines from "farhrplan_opt.patch".  Although I doubt it will end up back into simutrans or simutrans-extended, I would like to see the new scheduling features allow for such distribution methodologies, as they would not be uncommon for the real world.

void convoi_t::advance(){
    //code from fahrplan_opt.patch

    // Advance schedule
    if(loading_level == 0 || loading_level == 100 || get_no_load() || get_withdraw()){
        uint8 this_stop = fpl->get_aktuell();
        bool stop_here;

        do{
            stop_here = false;
            fpl->advance();

            if(is_waypoint(fpl->eintrag[fpl->get_aktuell()].pos)){
                stop_here = true;
                break;
            }
            if(loading_level==0){
                if(fpl->get_current_eintrag().ladegrad>=1)
                    stop_here = true;
            }else{
                for(unsigned i = 0; i<anz_vehikel && !stop_here; i++){
                    vehikel_t* v = fahr[i];
                    if (! v->get_fracht().empty()){
                        FOR(slist_tpl<ware_t>,ware,v->get_fracht()){
                            halthandle_t end_halt = ware.get_ziel();
                            halthandle_t this_halt = haltestelle_t::get_halt(fpl->eintrag[fpl->get_aktuell()].pos,besitzer_p);
                            if(end_halt == this_halt){
                                stop_here = true;
                                break;
                            }
                        }
                    }
                }
            }
        }while(!stop_here && fpl->get_aktuell() != this_stop);
        if(fpl->get_aktuell() == this_stop){
            fpl->advance();
        }
    }else{
        fpl->advance();
    }
}



On a separate note, I have given inflation and balancing of prices some thought and I would recommend that each cost (one-time, monthly, or per km) be stored in an array containing a breakdown of costs by unit.  Element zero would be the cost in simucents (for legacy purposes), and the remainder being up to the pakset designer, for example: British pounds, man-hours train driver, man-hours cook, man-hours mason, gallons oil, tons of coal, chords lumber, cubic yards dirt moved, etc.  The pakset would have a list of conversion rates from those units to simucents for various years, which would be interpolated and placed into an array at the start of each month.  Any accounting would then only need to multiply each element in the cost array by each element in the monthly conversion array and total all elements.

jamespetts

I have attempted to merge the latest changes from the master branch into the ex-15 branch, but this has failed as there are some very major merge conflicts. These appear mostly to relate to the UI, code with which I am unfamiliar, so I am not able to solve these conflicts, as I do not know the intention of much of the conflicted code.

Ranran, would you be able to look into the UI related merge conflicts? What I was hoping to do was to add the tool definitions for the consist orders so that you can start work on the UI for this, and also look into the error that you reported relating to vehicle data relating to inflation, but I cannot do that while these merge conflicts remain.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

I believe the conflict has been resolved. But as previously reported it already had some problems with boot so haven't tested this.

jamespetts

Quote from: Ranran on October 23, 2022, 05:08:51 PMI believe the conflict has been resolved. But as previously reported it already had some problems with boot so haven't tested this.

Thank you for this. I believe that I have now fixed the crash on loading relating to inflation data - thank you for spotting this.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

jamespetts

Quote from: Ranran on July 11, 2022, 04:17:43 PMThe latter.
It means adding a new empty constist_order_t (and editing or deleting it) to the schedule entry. I think it's the same as adding a new line/convoy to the world. I don't see anything like the definition of tool.
I have spent a little time looking into this. This is in fact already incorporated into the tool_change_line_t. This will copy the schedule and transmit it over the network. Consist orders are just data contained in the schedule, and there is specific code for copying and transmitting the consist order specific data using the ssprintf_schedule() and sscanf_schedule().

Thus, the UI just needs to create and populate the consist orders attached to a particular schedule, and then use the existing tool_change_line_t to transmit this over the network: the code should already be there for that.

I hope that this helps - please let me know if anything is unclear or if you think that anything is still missing.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

#191
It's been a while since I was working on it and I don't remember much, so it may take some time to resume working.
The UI mockup has some form, but I can't remember exactly what the intent was.

Quote from: jamespetts on October 30, 2022, 01:44:56 PMI have spent a little time looking into this. This is in fact already incorporated into the tool_change_line_t. This will copy the schedule and transmit it over the network. Consist orders are just data contained in the schedule, and there is specific code for copying and transmitting the consist order specific data using the ssprintf_schedule() and sscanf_schedule().

Thus, the UI just needs to create and populate the consist orders attached to a particular schedule, and then use the existing tool_change_line_t to transmit this over the network: the code should already be there for that.

I hope that this helps - please let me know if anything is unclear or if you think that anything is still missing.
What I mean is the command to generate a new consist_order_t, not the way to register it with line.
A few mockups of the non-functional UI have already been implemented.
Checking that might convey my intentions a little.
I don't mean a command that registers an edited consist order to a route, but a way to create an empty consist order for editing.
The UI cannot be edited consist_order_t as it has no container for data(consist_order_t).
Also, I don't think the schedule is always tied to the line.
I'm sorry, but I may have stumbled on a rudimentary part. No UI work has been done on consist_order so far...

jamespetts

Creating the consist order does not require a network tool, since it can be created locally on a client and then transmitted once it is applied to a schedule. I believe that consist orders are only available on schedules that belong to a line.

Were you referring to network tools or did you mean to refer to some other sort of tool? So far as I can see, all that the UI need to do is to call a constructor of the consist_order_t object, allow the user to modify data in that object and then use the line tool as described to commit it to a line (over the network if applicable).

Does this assist?

Did you need the get and set methods written?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

Quote from: jamespetts on November 01, 2022, 05:42:43 PMCreating the consist order does not require a network tool, since it can be created locally on a client and then transmitted once it is applied to a schedule. I believe that consist orders are only available on schedules that belong to a line.

Were you referring to network tools or did you mean to refer to some other sort of tool? So far as I can see, all that the UI need to do is to call a constructor of the consist_order_t object, allow the user to modify data in that object and then use the line tool as described to commit it to a line (over the network if applicable).

Does this assist?

Did you need the get and set methods written?
Thank you for your advice, this is very helpful.


QuoteDid you need the get and set methods written?
I expected consist_order_t to have a function to manipulate it. The lack of it seems to have stopped me. (I had the misconception that it required a call to the tool.)
I'm currently working on adding the missing functions.
Right now it's making little progress. I might ask you something if I get stuck again.



some thoughts for consist order:
- I suspect that the vehicle_description_element has flags for catering and must_carry_class and capacity, but is missing parameters for constraints on the goods category.
- I think that the vehicle_description_element needs a flag regarding whether the vehicle can be reversed and the vehicle orientation. A convoy coming from the opposite direction may appear in the opposite direction.
- I'm wondering if the vehicle_description_element needs a flag like if it has a cab in the front or rear or can be a tail. This may be a necessary condition when splitting trains. Also, I think that existing vehicle type diagrams can be used for constraints such as cab.


Ranran

#194
QuoteRight now it's making little progress. I might ask you something if I get stuck again.
Unfortunately, my mudboat sank shortly after departing from the dock, and got stuck as usual.
The consist order ui is now locally editable for the consist order. But I didn't understand how to commit it using with network tool.
I'm not familiar with the schedule edit code and you said tool_change_line_t but the tangle of schedule tools and line tools sank my ship.
Any help with this would be appreciated.


However, some UI work has progressed and I hope we can get some feedback at this point.
Currently, only simple operations can be performed. You can register the vehicle description to the order list using vehicle/convoy picker.
The Consistent Order UI is largely unfinished but newly designed and I'm curious how intuitive it will be for new players. Therefore, I omit the explanation about it at this stage. I would be happy to hear your opinion on this.

jamespetts

I wanted to look into this this afternoon, but, unfortunately, once I merged the master branch into the ex-15 branch, the game consistently crashes on startup when reading a pakset (specifically, when registering a wayobj_desc). It does this irrespective of whether it is reading a 14.x or 15.x pakset. I am not sure which particular set of changes caused this, but I suspect that it is linked to some of the merging from Standard, as none of the other recent changes on the master branch affect loading paksets. Incorporating your Ex-15-2211 changes did not alter this situation.

I am not going to be able to progress with this until this issue has been solved.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

jamespetts

I think that I have found and applied a fix for the above, although I am not sure that this deals with the underlying problem. Nevertheless, the game does now appear to work.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

jamespetts

I have now had a chance to look at the schedule UI. First of all, I have added some English translation texts for them. I will deal with the remaining matters in an enumerated list below.

Schedule UI

  • The overall layout is much better than the existing schedule window in 14.x.
  • The display does not seem to update properly when enabling/disabling a speed limit: I need to make some other change (e.g. modifying the wait for trigger) for this to update.
  • The symbol for "ignore choose signal on arrival" is confusing, as a lower case "i" in a circle usually means "information" - this may need a different symbol (perhaps a junction signal with a cross through it?)
  • The "modify_convoy" button is so wide that there is no padding between it and the right hand edge of the window.

I attempted to review the consist order UI, but unfortunately, this crashes on opening. The issue appears to be related to trying to write -1 to an unsigned integer, which integer is used as the index of a vector; the unsigned integer underflows to a very high number, which is then out of bounds for the vector.

This also exposes another problem on the ex-15 branch - the fatal error dialogue seems to have stopped working. Instead of showing the fatal error dialogue when trying to read an array beyond bounds, the game simply freezes.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

Thank you for your feedback
QuoteThe symbol for "ignore choose signal on arrival" is confusing, as a lower case "i" in a circle usually means "information" - this may need a different symbol (perhaps a junction signal with a cross through it?)
I believe this has been the case in the past as well. If the theme doesn't have a symbol for ignore_choose, an alternative symbol will be displayed.

Quote from: jamespetts on November 05, 2022, 08:29:30 PMI attempted to review the consist order UI, but unfortunately, this crashes on opening. The issue appears to be related to trying to write -1 to an unsigned integer, which integer is used as the index of a vector; the unsigned integer underflows to a very high number, which is then out of bounds for the vector.

consist_order_element_t& get_order(uint32 element_number)This was caused by passing a value greater than the possessed order to get_order. The crashing issue has been resolved.

QuoteThis also exposes another problem on the ex-15 branch - the fatal error dialogue seems to have stopped working. Instead of showing the fatal error dialogue when trying to read an array beyond bounds, the game simply freezes.
I have no idea why the merge caused this error to cause a crash and not even display a fatal error...

Ranran

Quote from: jamespetts on November 05, 2022, 08:29:30 PMI attempted to review the consist order UI, but unfortunately, this crashes on opening. The issue appears to be related to trying to write -1 to an unsigned integer, which integer is used as the index of a vector; the unsigned integer underflows to a very high number, which is then out of bounds for the vector.

This also exposes another problem on the ex-15 branch - the fatal error dialogue seems to have stopped working. Instead of showing the fatal error dialogue when trying to read an array beyond bounds, the game simply freezes.
I checked the branch which incorporated two branches about merging from standard into the ex15 branch.
https://github.com/Ranran-the-JuicyPork/simutrans-exp/tree/ex15-srd10368
https://github.com/Ranran-the-JuicyPork/simutrans-exp/tree/ex15-std10719
However, I was unable to reproduce this symptom on these branches. Another change on the master branch may be involved.
ex) csv branch reversion, modifying project files etc.

jamespetts

Excellent, thank you for that. Some further feedback follows.

Schedule UI

  • The tooltip for the speed limit would work better attached to the "enable speed limit" button rather than the speed limit number input, I think.
  • It would be better for the "Speed limit:" text to be grey if "enable speed limit" has not been set.
  • There needs to be a space between "Speed limit:" and the decrement button for the speed limit adjustment button.
  • The "Modify consist" button now has a better width.
  • Modifying the speed limit still does not update the schedule display on the left of the window until something else has changed.
  • I have modified reference to "convoy" to "consist" - I should be grateful if all future references to "convoy" could be changed to "consist", as I intend to make this consistent throughout the UI from version 15.x onwards ("convoy" has always been a confusing and inaccurate name).
  • We are missing a UI for the target_id_condition_trigger, target_id_couple, target_id_uncouple and target_unique_entry_uncople data. These data are intended to be used for schedule bifurcation and concatenation.

Consist order UI

  • I have similarly modified "convoy" to "consist".
  • The box for the vehicle picker has no padding at the lower and left-hand edges.
  • The current behaviour seems to be in the vehicle picker to include nothing other than locomotives in the list until a filter has been selected. This is likely to be confusing for players. I suggest including everything unless a filter be selected.
  • The vehicles displayed in the add vehicles box seem to exclude many items that should not be excluded. For example, in the Pak128.Britain-Ex demo game, creating a new consist order for a railway schedule will show only the LMS 8F locomotive and tender with no type of goods selected. In fact, there are many other types of locomotives available.
  • The detail display to the right for vehicles in the vehicle picker looks very good and is easy to understand.
  • It would be helpful if the vehicle picker could automatically prevent invalid combinations in the same way as the depot window does.
  • It would be helpful if the vehicle picker could show whole consist data (e.g., maximum speed, power, weight, etc). for the consist as modified
  • It would be helpful if the consist order window could include a simple one line explanation somewhere for what players are supposed to be doing, something like, "Select the vehicles that you want to make up this consist at this stop and onwards on this schedule".
  • It would be helpful if the consist order window would automatically start with the consist that is already on the line (and, if there is more than one type, the most common type on that line, or, if none are more common than others, the first in order) to make it clearer to the player what is to be done and to reduce workload.

As to this:

QuoteBut I didn't understand how to commit it using with network tool.
I'm not familiar with the schedule edit code and you said tool_change_line_t but the tangle of schedule tools and line tools sank my ship.
Any help with this would be appreciated. 

The basic idea is that the consist order is just another datum in the schedule: you create the consist order on the stack (i.e., as a local variable, not using the "new" command) and then just assign (copy) the consist order object to the inthashtable representing the consist order for the particular entry in the schedule to which it relates. Then, you commit the changes to the schedule in the same way as you would commit a simpler change (e.g., adding or removing an entry).

If, after the above explanation, you are still having difficulty, what I think would be the easiest is if you add a place in the UI code where the consist order should be committed to the schedule, let me know where that is, and I will see if I can code the actual committing to the schedule part. Would this be workable?

Thank you for your work on this: it is much appreciated.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

#201
Quote from: jamespetts on November 06, 2022, 12:24:02 PMThe basic idea is that the consist order is just another datum in the schedule: you create the consist order on the stack (i.e., as a local variable, not using the "new" command) and then just assign (copy) the consist order object to the inthashtable representing the consist order for the particular entry in the schedule to which it relates. Then, you commit the changes to the schedule in the same way as you would commit a simpler change (e.g., adding or removing an entry).

If, after the above explanation, you are still having difficulty, what I think would be the easiest is if you add a place in the UI code where the consist order should be committed to the schedule, let me know where that is, and I will see if I can code the actual committing to the schedule part. Would this be workable?

void consist_order_frame_t::save_order()
{
    schedule->orders.remove(unique_entry_id);
    schedule->orders.put(unique_entry_id, order);
}
I think I've succeeded in temporarily committing the order to the schedule.
Edits are retained until the dialog is closed.
But I don't know how to add a process to save the schedule's orders, so the contents of the orders are lost when the dialog is closed.

What I came up with was to embed the following code in bool line_management_gui_t::infowin_event(), but it didn't work.
                for (auto order : schedule->orders) {
                    tool_t *tool = create_tool(TOOL_CHANGE_LINE | SIMPLE_TOOL);
                    cbuffer_t buf;
                    buf.printf("g,%i,", line.get_id());
                    order.value.sprintf_consist_order(buf);
                    tool->set_default_param(buf);
                    welt->set_tool(tool, line->get_owner());
                    // since init always returns false, it is save to delete immediately
                    delete tool;
                }


QuoteIf, after the above explanation, you are still having difficulty, what I think would be the easiest is if you add a place in the UI code where the consist order should be committed to the schedule, let me know where that is, and I will see if I can code the actual committing to the schedule part.
Unfortunately I don't think I understand how to store the consist order datum correctly.
I suppose the edited orders are stored correctly in the schedule_t. I think line_management_gui_t::infowin_event is the place to do the saving process.



QuoteThe vehicles displayed in the add vehicles box seem to exclude many items that should not be excluded. For example, in the Pak128.Britain-Ex demo game, creating a new consist order for a railway schedule will show only the LMS 8F locomotive and tender with no type of goods selected. In fact, there are many other types of locomotives available.
To make vehicle selection easier for the player, the vehicle picker only lists vehicles owned by the player.
I don't think you need to list vehicles you don't own.
The vehicle operation plan starts from where player purchase the vehicle first in the depot. This allows players to avoid accidentally placing non-executable consist orders.

jamespetts

Thank you for this. Can you commit your work so far so that I can work from there?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

Quote from: jamespetts on November 06, 2022, 02:24:17 PMThank you for this. Can you commit your work so far so that I can work from there?
I pushed a testing commit.

Ranran

Quote from: jamespetts on November 06, 2022, 12:24:02 PM
  • The tooltip for the speed limit would work better attached to the "enable speed limit" button rather than the speed limit number input, I think.
  • It would be better for the "Speed limit:" text to be grey if "enable speed limit" has not been set.
  • There needs to be a space between "Speed limit:" and the decrement button for the speed limit adjustment button.
These changes are complete.


Quote from: jamespetts on November 06, 2022, 12:24:02 PMWe are missing a UI for the target_id_condition_trigger, target_id_couple, target_id_uncouple and target_unique_entry_uncople data. These data are intended to be used for schedule bifurcation and concatenation.
Indeed these parameters seem to exist in the code, but I searched the forums and didn't find any explanation for this.
I'm not exactly sure how to set what.
For example what does target_id refer to? schedule? line? How then does the player obtain it? For example, select from a list. Or do we have to type the ID number directly into the text box?

jamespetts

Quote from: Ranran on November 06, 2022, 02:33:16 PMI pushed a testing commit.
Thank you for this. I think that I have managed to fix the issue with the schedule changes not committing - can you test this to check that this is working? If that is working, is there anything else that I need to do before you can continue work on the consist order dialogue?

I will reply to your other questions when I have a little more time.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Ranran

#206
Quote from: jamespetts on November 06, 2022, 09:40:05 PMThank you for this. I think that I have managed to fix the issue with the schedule changes not committing - can you test this to check that this is working? If that is working
I am afraid but it's still not working. (´・ω・`)
What I did find out was that schedule_t::matches() always returned false.
I think this is mainly because check_consist_orders_for_match() was not returning the correct value. I think i fixed it. But it still doesn't seem to save schedule_t::orders.
Does code that uses sprintf_consist_order need to be added somewhere?

Ranran



QuoteThe current behaviour seems to be in the vehicle picker to include nothing other than locomotives in the list until a filter has been selected. This is likely to be confusing for players. I suggest including everything unless a filter be selected.
changed as such.

Quote from: jamespetts on November 06, 2022, 12:24:02 PM
  • It would be helpful if the consist order window could include a simple one line explanation somewhere for what players are supposed to be doing, something like, "Select the vehicles that you want to make up this consist at this stop and onwards on this schedule".
I added this to the top of the UI, do you think this is the best place for it?


QuoteIt would be helpful if the consist order window would automatically start with the consist that is already on the line (and, if there is more than one type, the most common type on that line, or, if none are more common than others, the first in order) to make it clearer to the player what is to be done and to reduce workload.
- Changed the convoy picker tab to the first tab.
- Added line filter and turned it on by default.

ceeac

Regarding the issues with consist_order_t, there is this piece of code in consist_order_t.h:
consist_order_element_t& get_order(uint32 element_number)
{
if (element_number > orders.get_count()) { return consist_order_element_t(); }
return orders[element_number];
}
which is invalid:
bauer/../tpl/../dataobj/../dataobj/consist_order_t.h:237:53: error: non-const lvalue reference to type 'consist_order_element_t' cannot bind to a temporary of type 'consist_order_element_t'
                if (element_number > orders.get_count()) { return consist_order_element_t(); }
                                                                  ^~~~~~~~~~~~~~~~~~~~~~~~~

jamespetts

Ranran - thank you for that. I am having some trouble merging your ex15-2211 branch, however, in that I get the following merge error in simwin.h:

<<<<<<< HEAD
+ magic_script_error,
  =======
+ magic_consist_order_rdwr_dummy, // only used to load/save
  >>>>>>> 3b326559462272a0585b8ab91afeafa50c644460
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.