Hi James,
I wonder how this works exactly:
A passenger enters a vehicle, lets say it is a p_class[3] passenger enters a p_class[3] class, but the compartment is a reassigned p_class[2] compartment.
Does the passenger know it is sitting in a p_class[2] compartment, or does it only know that it is traveling in a p_class[3] class?
I would very much like to know in which COMPARTMENT the passenger travels, so I can display it correctly in the convoy details window. Currently, I can not get that functionality to work (if it is even possible) with the result of either the passenger manifest disappear when changing class, doubles up when a vehicle has two compartments with the same class, or nothing is displayed at all.
Also, how does the passenger know which compartment to put himself in when two compartments have the same class but different comfort? I would asume that the passengers would put them self in the compartment with the highest comfort in such cases, but they appear to sit a bit random.
Also, I think there is a bug in this part of the simvehicle.cc:
uint16 vehicle_t::get_total_cargo_by_class(uint8 g_class) const
{
// Take into account class reassignments.
uint16 carried = 0;
for (uint8 i = 0; i < desc->get_number_of_classes(); i++)
{
if(class_reassignments[i] == g_class && desc->get_capacity(i) > 0)
{
carried += fracht[i].get_count();
}
}
return carried;
}
It does not produce any number, and comparing it with the get_freight_info(), I have tried with this instead, which appears to work:
uint16 carried = 0;
if (desc->get_capacity(g_class) > 0)
{
if (!fracht[get_reassigned_class(g_class)].empty())
{
FOR(slist_tpl<ware_t>, const& ware, fracht[get_reassigned_class(g_class)])
{
carried += ware.menge;
}
}
}
return carried;
although that creates the errors I mentioned above.
There is a new version, available here: https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/passenger-and-mail-classes-gui-combolist (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/passenger-and-mail-classes-gui-combolist)
However, the convoy details window is currently a bit buggy with flashing displays. I wont fix that now until I have resolved the issues I mentioned above.
Quote from: Ves on August 24, 2017, 12:02:46 AM
Hi James,
I wonder how this works exactly:
A passenger enters a vehicle, lets say it is a p_class[3] passenger enters a p_class[3] class, but the compartment is a reassigned p_class[2] compartment.
Does the passenger know it is sitting in a p_class[2] compartment, or does it only know that it is traveling in a p_class[3] class?
If things are working correctly (if there are errors, I should be grateful if you could post a full bug report), the reassignment means that the original (default) class designation is irrelevant, and all that is detected by either passenger choice or revenue calculation is the new, reassigned class.
However, if you are writing new code interfacing with this, you will need to make sure to use the class reassignments array every time that you access vehicle class data (I cannot remember immediately whether this is already automatically implicit in methods returning class data - I think that it may well be in at least some cases, but you should check).
QuoteI would very much like to know in which COMPARTMENT the passenger travels, so I can display it correctly in the convoy details window. Currently, I can not get that functionality to work (if it is even possible) with the result of either the passenger manifest disappear when changing class, doubles up when a vehicle has two compartments with the same class, or nothing is displayed at all.
When a vehicle has two sets of accommodation ("compartments"; but this is misleading, as the term "compartment" has a very specific meaning in the context of railway carriages which is not consistent with usage here) of the same class (which will only happen when one or more has been reassigned), surely the passengers should all simply be shown as being in accommodation of that class? I did not set up the data structures to distinguish between accommodation that was originally of, say, class 3 reassigned to class 2 and accommodation that was originally of class 2. All that is relevant is the currently assigned class of accommodation.
QuoteAlso, how does the passenger know which compartment to put himself in when two compartments have the same class but different comfort? I would asume that the passengers would put them self in the compartment with the highest comfort in such cases, but they appear to sit a bit random.
There is no decision mechanism here, and it is indeed random, just as it was before classes were introduced when a train had carriages of different levels of comfort: the comfort levels were simply averaged. Bear in mind that only an experienced passenger will work out which carriages are more comfortable in that situation.
Quote
Also, I think there is a bug in this part of the simvehicle.cc:
uint16 vehicle_t::get_total_cargo_by_class(uint8 g_class) const
{
// Take into account class reassignments.
uint16 carried = 0;
for (uint8 i = 0; i < desc->get_number_of_classes(); i++)
{
if(class_reassignments[i] == g_class && desc->get_capacity(i) > 0)
{
carried += fracht[i].get_count();
}
}
return carried;
}
It does not produce any number, and comparing it with the get_freight_info(), I have tried with this instead, which appears to work:
uint16 carried = 0;
if (desc->get_capacity(g_class) > 0)
{
if (!fracht[get_reassigned_class(g_class)].empty())
{
FOR(slist_tpl<ware_t>, const& ware, fracht[get_reassigned_class(g_class)])
{
carried += ware.menge;
}
}
}
return carried;
although that creates the errors I mentioned above.
The error appears to be the counting of the number of packets of passengers/mail without taking into account the number of units inside those packets. I have pushed a fix for this error, and used this corrected method instead of the "class_compartment" method that you had written. I have not had a chance to test this other than briefly, however: I should be grateful if you could check whether this works.
I should note, however, that the method as I had originally written it did produce numbers: just not quite the right numbers as described above.
Quote
There is a new version, available here: https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/passenger-and-mail-classes-gui-combolist (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/passenger-and-mail-classes-gui-combolist)
However, the convoy details window is currently a bit buggy with flashing displays. I wont fix that now until I have resolved the issues I mentioned above.
Thank you for this. I cannot see any flashing displays, but I do notice that the passengers are not properly summed/condensed in their descriptions, e.g.
"2 passengers > King's Cross
3 passengers > King's Cross
1 passenger > King's Cross
2 passengers > Finsbury Park
4 passengers > Finsbury Park"
instead of
"6 passengers > King's Cross
6 passengers > Finsbury Park"
Thank you very much for your ongoing work on this, incidentally: it is much appreciated.
Dear Jamespetts,
When passenger classes are implemented in Simutrans, how will single-class vehicles be handled? Will there be a special parameter that allows vehicles like city buses to carry passengers from any class in varying quantities, or will first class passengers not take short distance transport at all, instead travelling to long-distance rail stations/airports by car? Also, is the implementation of classes intended to resolve the current unrealistic situation whereby every passenger can afford air travel on any route? For example, if I set up a flight and a long-distance coach line between the same two stations currently, nobody will ride the coach. If, however, Economy Class air travel was coded in as one class higher than the bus, people would use both forms of transport. Is this intended to be the case?
Thank you,
Passengerpigeon.
Lower-class vehicles will be able to carry higher-class passengers. Avoiding every passenger travelling solely by air is the very purpose of the feature.
As Rollmaterial wrote, passengers of a higher class will always be able to travel in lower class transport without any special parameters. By default, 'buses will be configured to accept the lowest class of passengers, so all will be able to travel.
Higher class passengers will be more likely to have access to a car. At present, private cars can only be used to make complete journeys. However, a car parking/park and ride feature is planned (although it may take some time to implement unless more people are able to assist with coding) that will allow passengers to make part of their journey by car and part of their journey by player transport.
As to the latter question, the simple answer is "yes".
New version here (created new github branch): https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes)
Quote from: jamespetts on August 24, 2017, 12:58:32 AM
If things are working correctly (if there are errors, I should be grateful if you could post a full bug report), the reassignment means that the original (default) class designation is irrelevant, and all that is detected by either passenger choice or revenue calculation is the new, reassigned class.
However, if you are writing new code interfacing with this, you will need to make sure to use the class reassignments array every time that you access vehicle class data (I cannot remember immediately whether this is already automatically implicit in methods returning class data - I think that it may well be in at least some cases, but you should check).
When a vehicle has two sets of accommodation ("compartments"; but this is misleading, as the term "compartment" has a very specific meaning in the context of railway carriages which is not consistent with usage here) of the same class (which will only happen when one or more has been reassigned), surely the passengers should all simply be shown as being in accommodation of that class? I did not set up the data structures to distinguish between accommodation that was originally of, say, class 3 reassigned to class 2 and accommodation that was originally of class 2. All that is relevant is the currently assigned class of accommodation.
Well, yes you are right that all passengers show up as "that class", however, since we now have two accommodations with the same class, the passenger manifest for both accommodations shows the combined capacity. get_capacity_by_class() doesnt think of accommodation at all what I can see?
If you change a class on a running vehicle (open convoy detail and click the new button class manager), the passenger manifest will either disappear, or merge with similiar entries, dependent on wether you set it to a new class or a similar class as an existing one.
Quote
There is no decision mechanism here, and it is indeed random, just as it was before classes were introduced when a train had carriages of different levels of comfort: the comfort levels were simply averaged. Bear in mind that only an experienced passenger will work out which carriages are more comfortable in that situation.
Quote
Thank you for this. I cannot see any flashing displays, but I do notice that the passengers are not properly summed/condensed in their descriptions, e.g.
"2 passengers > King's Cross
3 passengers > King's Cross
1 passenger > King's Cross
2 passengers > Finsbury Park
4 passengers > Finsbury Park"
instead of
"6 passengers > King's Cross
6 passengers > Finsbury Park"
Thank you very much for your ongoing work on this, incidentally: it is much appreciated.
Thanks, I will look into that!
Quote from: Ves on August 26, 2017, 06:23:12 AM
Well, yes you are right that all passengers show up as "that class", however, since we now have two accommodations with the same class, the passenger manifest for both accommodations shows the combined capacity. get_capacity_by_class() doesnt think of accommodation at all what I can see?
If you change a class on a running vehicle (open convoy detail and click the new button class manager), the passenger manifest will either disappear, or merge with similiar entries, dependent on wether you set it to a new class or a similar class as an existing one.
This is intended, as there is no use in a player having to add up two different entries of the same class to see how many passengers of any given class are travelling: this is similar in a way to the second issue reported with multiple passengers for the same destination.
Quote from: jamespetts on August 26, 2017, 10:38:06 AM
This is intended, as there is no use in a player having to add up two different entries of the same class to see how many passengers of any given class are travelling: this is similar in a way to the second issue reported with multiple passengers for the same destination.
Hmm, to be honest, I dont think it matters that the player have to add up different entries of the same class to see how many passengers of a given class are travelling in one vehicle. Showing the amount of same class passengers on a per vehicle basis, I think is fine if it is not possible on a per accommodation, but it is much less overview friendly I think. Remember that we need to show both which class the passengers are traveling in as well as the class the passengers MAY be traveling in. I do plan to show the sum of alll travelers of a given class in the cargo manifest in the convoy window, so this amount should already be known. As this is the detailed convoy window, detailed information is appreciated, I would consider this to be worth showing.
I do have to rethink the design if it wont be possible. As you can see in the current convoy details window, I wanted to show the manifest in the accommodations sections, which would mean:
It saves space in the window, since I otherwise have to add a manifest section below the accommodations section as well as spacing sections to keep an overview.
It would not require duplicate information, like how many passengers a class can hold (ok, semi true, since you would have to manually add up the amounts in the accommodations), or which class the passengers are traveling on (we would have to show the class names again). It can quickly become alot of "class" names around, just look at these screenshots (the pictures are captured at different times, so the manifest is different):
combined class manifest:
(http://server.exp.simutrans.com/screenshots/convoy_detail%20passenger%20manifest.png)
I did just mock it together in msvs, and I just noticed there is a bug in the code, which makes the entries show twice. It might be a little more streamlined, like not showing a class if it is not used.
separated class manifest:
(http://server.exp.simutrans.com/screenshots/convoy_detail%20passenger%20manifest_2.png)
This is how I would want it to look. It works in all cases, except if two accommodations are the same.
Another thing to consider is this:
When a class is reassigned, all travelers from that class turns invincible, which is because they are shown per class, so when changing the class to something else, we thereby are not showing passengers of that old class any more.
I dont know how it is treated in the code when you change the class on the fly. Passengers obviously keep their old class, but will they get of the train at the next stop or will they keep their seat for the entire jurney?
One can think of multiple solutions to this:
Easiest solution is to remove the class manager button from the vehicle details window, so you cant reassign classes outside the depot, but that is a shame I think.
Alternatively, if we make it possible to show the manifest on a per accommodation basis, a check would be run and a text appear in the manifest that the class is reassigned but current passengers keep their class
If it wont be possible to show the manifest on a per accommodation basis, we could still check if there are some passengers that are traveling on another class than what should be available, showing both the passengers and the previously mentioned text in the manifest.
Quote from: Ves on August 26, 2017, 06:16:35 PM
Hmm, to be honest, I dont think it matters that the player have to add up different entries of the same class to see how many passengers of a given class are travelling in one vehicle. Showing the amount of same class passengers on a per vehicle basis, I think is fine if it is not possible on a per accommodation, but it is much less overview friendly I think. Remember that we need to show both which class the passengers are traveling in as well as the class the passengers MAY be traveling in.
Would this not adequately be achieved by showing the actual class of the passengers? Although I should note that this information would not ordinarily be available to a transport provider, of course: the difference between those who are able to pay more but choose not to do so and those who are unable to pay more is usually invisible to a business, although I note that it is customary in computer games to provide the player with more information than one would normally expect to have.
I should note that the idea always was that the default assignment of accommodation of vehicles to classes was always intended to be no more than a suggestion to the players in any event - a reasonable default that a sensible player would be most likely to choose in most situations. The idea is simply to minimise the amount of work that a player needs to do compared with, for example, having all accommodation always at class 0 by default, or having it in some indeterminate state which players would then have to customise manually before using the vehicles (and it was easier to program than the latter, too).
Therefore, showing the original, default assignment of classes to different parts of a vehicle's accommodation would not seem to be particularly useful; what use had you intended for this information, other, perhaps, than showing what pressing "reset all classes" will do?
However, am I missing something, or have you already got the original default classes shown in the first of the two screenshots below...? I must confess, I am a little unclear on what exactly you want to achieve.
QuoteI do have to rethink the design if it wont be possible. As you can see in the current convoy details window, I wanted to show the manifest in the accommodations sections, which would mean:
It saves space in the window, since I otherwise have to add a manifest section below the accommodations section as well as spacing sections to keep an overview.
It would not require duplicate information, like how many passengers a class can hold (ok, semi true, since you would have to manually add up the amounts in the accommodations), or which class the passengers are traveling on (we would have to show the class names again). It can quickly become alot of "class" names around, just look at these screenshots (the pictures are captured at different times, so the manifest is different):
combined class manifest:
(http://server.exp.simutrans.com/screenshots/convoy_detail%20passenger%20manifest.png)
I did just mock it together in msvs, and I just noticed there is a bug in the code, which makes the entries show twice. It might be a little more streamlined, like not showing a class if it is not used.
separated class manifest:
(http://server.exp.simutrans.com/screenshots/convoy_detail%20passenger%20manifest_2.png)
This is how I would want it to look. It works in all cases, except if two accommodations are the same.
I must confess, I am having trouble following this. Is the second screenshot what you are trying to achieve? Am I missing something, or is the only difference layout? I am afraid that I am having trouble spotting what is different in terms of the actual information in the second version.
Quote
Another thing to consider is this:
When a class is reassigned, all travelers from that class turns invincible, which is because they are shown per class, so when changing the class to something else, we thereby are not showing passengers of that old class any more.
Surely this is how it would be for a real transport business, as explained above?
QuoteI dont know how it is treated in the code when you change the class on the fly. Passengers obviously keep their old class, but will they get of the train at the next stop or will they keep their seat for the entire jurney?
The passengers will not move once they have entered a particular class in the current code. It occurs to me that this could lead to an exploit of players changing the class of accommodation just before convoys reach their stop to increase the revenue. This cannot be fixed by moving the passengers to a lower class of accommodation, as the convoy might no longer have a lower class of accommodation.
Careful consideration is necessary as to how to fix this exploit: perhaps the increasing of a class should not be able to be done when there are passengers in the accommodation and there is no lower classed accommodation to which they can move.
Quote
One can think of multiple solutions to this:
Easiest solution is to remove the class manager button from the vehicle details window, so you cant reassign classes outside the depot, but that is a shame I think.
I agree that this should be retained if possible, subject to remedying the exploit as described above. It would be most inconvenient for players not to be able to do this outside a depot.
Quote
Alternatively, if we make it possible to show the manifest on a per accommodation basis, a check would be run and a text appear in the manifest that the class is reassigned but current passengers keep their class
If it wont be possible to show the manifest on a per accommodation basis, we could still check if there are some passengers that are traveling on another class than what should be available, showing both the passengers and the previously mentioned text in the manifest.
Because we do not know whether any given passenger travelling in any given class of accommodation would have purchased a higher class ticket if he/she could have afforded to do so, I suggest treating all passengers in a given class of accommodation as being not in a position to pay a higher fare than that associated with that class for the remainder of their journey, whatever their underlying class. This is the most realistic solution, I think (passengers would buy their tickets in advance, after all) and one that is the least prone to exploits.
I think I can see what you mean:
You mean that it might not be necessary to show the individual accommodations in the vehicle details window at all? In that case, I follow what you mean and there is indeed no point of showing passengers on a per accommodation basis.
However, just to clear somethings out as I might have been unclear:
The two screenshoots are different:
The first image was my suggestion on how to show a combined manifest below the accommodations section.
The second image was my idea of how I wanted it to look. You can see there is a manifest per accommodation, although there where no p_class[1] passengers at the time of the screenshoot. If there where, there would be a manifest below the p_class[1] entry as well.
The amount of information are the same in the two screenshoots, it is only the layout that is different. My personal view is that the first screenshoot is too cloggy, it is difficult to see what is what and relative little information takes alot of space, if you understand what I mean.
The problem with my idea, and the reason to this discussion is, I did not succeed for all circumstances, namely when the two compartments shares the same class.
But I will try and make a version where there is no information about specific accommodations of a vehicle, only which classes are held by a vehicle. I might add a note about how many compartments is in the vehicle, as I still believe this is usefull information for a player.
As to the other issue with reassigning classes on the fly, since passengers apperently keep traveling on the same class as they entered the vehicle, there would be no exploit?
My problem with it was that the passenger manifest for the compartment would be empty when reassigned, but this should not be an issue when I make the new window I think.
I have a new window design which I am happy with, and I am having some real brainexcercises by trying to figure out how some of the things work (including spelling to brain excercises!)
I have another suggestion for the get_total_cargo_by_class() in vehicle_t. The previous did not produce consistent results I think, so I have looked at how it is done in the simconvoy.cc for the get_freight_info() and made some modifications.
Could you have a look and see if you think this is sensible?
uint16 vehicle_t::get_total_cargo_by_class(uint8 g_class) const
{
uint16 carried = 0;
{
for (uint8 j = 0; j < desc->get_number_of_classes(); j++)
{
if (j == g_class)
{
FOR(slist_tpl<ware_t>, ware, fracht[j])
{
if (ware.menge != 0)
{
carried += ware.menge;
}
}
}
}
}
return carried;
}
edit: added brackets {} that where missing and removed wrong comment // in the code
Thank you for your work on this.
Looking at the second post first, I am not sure that I understand what this code is intended to achieve: it does not have any regard to the class reassignments (despite the comment that it does have regard to that remaining in place). So, if a vehicle's class 4 accommodation had been reassigned to class 3, for example, the method above would incorrectly produce a non-zero number when the parameter g_class passed to it equalled 4. In fact, the vehicle would have no class 4 accommodation because the accommodation that was by default class 4 had been reassigned to class 3, so it should return zero in this case. That is the purpose of the line:
if(class_reassignments[i] == g_class && desc->get_capacity(i) > 0)
We need to use
class_reassignments[i]
rather than just i alone in order to match the class as reassigned correctly. Does this make sense? I wonder whether what might be a misunderstanding about how class reassignments are intended to work might have affected your position in respect of the earlier post, too?
Just to note, however, I had misplaced a pair of brackets, which should be after the line "if (j == g_class)".
We should already know which class we want to see the cargo for. Therefore, using the class_reassignments is redundant, since that has already been done on beforehand when calling the tool. The comment was an old one which I had forgot to remove.
Since the name "get_total_cargo_by_class(g_class)" implies that we can get the amount of cargo from any class that is put in as g_class, it is also not good I think to tie it to the desc->get_capacity(i), or even just get_capacity(i) (without the "desc->") because if the class has been reassigned, the entry will show 0. That was my problem earlier, that I could not just simply ask for "give me class [2] passengers" even though there would be no class [2] compartment. With this implementation I can do that.
I was however quite puzzled when I experimented with this:
the fracht[ i ] appears not to work with the maximum possible amount of classes of the goodtype, but only with the max amount of classes for the vehicle. Therefore I am using the
desc->get_number_of_classes() in the for loop, even though i feel it should really have been the maximum number of classes for either passenger or mail, but when I use that in the for loop, it crashes when I try to call a higher class than what is in the vehicle.
In the current implementation, I noticed that there might be some water rings effect of having this change, for instance, I have problems when reassigning a p_class[0] to anything higher. It wont take any load. I guess that might be because I use the "desc->get_number_of_classes()".
Can the fracht[ i ] perhaps be modified to work on the max number of classes in general, instead of per vehicle?
If this all these changes in get_total_cargo_by_class() would create other rings in the water (which I dont know of), maybe have two different versions of it? One which is your original, and one which is my version?
I still think that there may be some confusion here as to how class reassignments are intended to work. I also note an error of my own in the algorithm for loading passengers, which I have just fixed, which might have accounted for at least some of the confusion. Perhaps a worked example will help. Let us imagine an aircraft with two classes: tourist class and first class. By default, tourist class is class 2 and first class is class 4. The array of capacities would be thus:
capacity(0) = 0
capacity(1) = 0
capacity(2) = 100
capacity(3) = 0
capacity(4) = 20
By default, our class reassignments array would be
class_reassignments[0]=0
class_reassignments[1]=1
class_reassignments[2]=2
class_reassignments[3]=3
class_reassignments[4]=4
Now, suppose that we want to reassign class 4 to class 3 and class 2 to class 1. Our class reassignments array would now be:
class_reassignments[0]=0
class_reassignments[1]=1
class_reassignments[2]=1
class_reassignments[3]=3
class_reassignments[4]=3
The capacities should thus be:
capacity(0) = 0
capacity(1) = 100
capacity(2) = 0
capacity(3) = 20
capacity(4) = 0
Does this make sense? Have you found that it does not, in fact, work like this, or did you not understand that it should work like this?
I should note, incidentally, that I am going on holiday to-morrow for just under a week, so I might not be able to respond in much detail until my return.
Thanks for the explanation! I think I understand it and it seems to make sense.
I think that also works as expected from what I have seen.
What I am looking for is this, and I will use your example:
Originally the plane have this capacity:
Capacity(0) = 0
Capacity(1) = 0
Capacity(2) = 100
Capacity(3) = 0
Capacity(4) = 20
Now the plane is filled up to 100% load into those classes. During the middle of the cruise, you decide to reassign the classes as you described.
The passengers, however, are still traveling on their original classes, and should be found like this:
Get_cargo_by_class(0) = 0
Get_cargo_by_class(1) = 0
Get_cargo_by_class(2) = 100
Get_cargo_by_class(3) = 0
Get_cargo_by_class(4) = 20
But I cannot make this work in the original implementation of Get_cargo_by_class(). Instead, it requires the class I am checking for cargo to also be a class with a capacity more than 0.
This is where my problem is at.
There are no variables set aside for remembering what class that any given accommodation was before the last reassignment, or whether any given passenger boarded before or after any reassignment took place. That would be extremely complex to implement and might impact performance.
Instead, if classes are re-assigned when passengers are in transit, the passengers will need to move classes if they can to match the class that they boarded. If they cannot move classes, when the revenue is calculated, the minimum of the passengers' underlying classes and the class (as reassigned) of their current accommodation should be used.
Does this make sense?
ah, ok then I think I have misunderstood something. I was under the impression that the passengers would keep their class even though you reassigned it by this comment
Quote from: jamespetts on August 27, 2017, 09:21:59 PM
The passengers will not move once they have entered a particular class in the current code.
If that is not the case, I am afraid that I do not dare to code anything that is beyond the GUI parts, as it would be very likely to spawn errors that I dont understand. Otherwise, do you have any suggestions as to which elements of the code I need to use to find the "hidden" passengers?
I may have been unclear about what happens to passengers when the class is reassigned: my apologies. As things currently stand,
(1) the revenue per passenger is based on the class of accommodation in which they are situated when they arrive at their departure point; and
(2) when the class of reaccommodation is re-assigned, no changes are made to the passengers who are travelling.
There are thus never any hidden passengers.
However, this is a problem, as this can give rise to an exploit, allowing players to increase the class during a journey, permitting players to earn more money from passengers than their class permits.
I had suggested a system in which passengers would be moved to lower class accommodation (if available) if the class increases during their journey, and that in any event, the revenue for any given passenger should never exceed that of the passenger's underlying class. This has, however, yet to be implemented.
Does this clarify anything?
Ok, but I don't understand how it is an exploit if the passengers who are traveling keep their class if you reassign it just before the arrival. Or is it plainly what the accommodation class at arrival that decides how much money is made?
It is at present just the class of accommodation that determines the revenue (if I remember the code correctly, in any event).
Thank you, now I understand! I will wait with coding further gui elements that is about the cargo hold then and perhaps continue with the class proportion for buildings etc.
Splendid. I am currently in Switzerland, so will not be able to do any coding until next week end.
I am having a bit of a puzzle with the class proportions stuff for the buildings.
I have found so far get_class_proportion(i) and get_class_proportion_jobs(i), which I think are supposed to determine how likely a passenger from every class (i) is to be spawned.
Together with that I have found get_number_of_class_proportions() and get_number_of_class_proportions_jobs(), which only seems to count the amount of "proportion entries" that the building possess.
Then there is the get_class_proportions_sum() and get_class_proportions_sum_jobs(), which I dont really can lure what they do.
Now I try to put everything together in the building info window to see how it looks like. The layout for the information should be something else in the end.
The right window appears to look like expected, but the left window have some weird stuff going on. It appears as it is the get_class_proportion_jobs(i) that is off, and I noted that every single building, except the one owning the infowindow to the right, is like that, but with different random numbers.
(http://simutrans-germany.com/files/upload/simscr28.png)
I have used the code like this:
// class entries
char p_class[32] = "\0";
char m_class[32] = "\0";
building_desc_t const& h = *tile->get_desc();
if (h.get_class_proportions_sum() > 0)
{
buf.printf("%s: %d\n", translator::translate("class_proportions_sum"), h.get_class_proportions_sum());
}
if (h.get_class_proportions_sum_jobs() > 0)
{
buf.printf("%s: %d\n", translator::translate("class_proportions_sum_jobs"), h.get_class_proportions_sum_jobs());
}
for (int i = 0; i < h.get_number_of_class_proportions(); i++)
{
if (h.get_class_proportion(i) > 0)
{
sprintf(p_class, translator::translate("p_class[%u]"), i);
buf.printf("%s %s: %d\n", translator::translate("class_proportion"), p_class, h.get_class_proportion(i));
}
}
for (int i = 0; i < h.get_number_of_class_proportions_jobs(); i++)
{
if (h.get_class_proportion_jobs(i) > 0)
{
sprintf(p_class, translator::translate("p_class[%u]"), i);
buf.printf("%s %s: %d\n", translator::translate("class_proportion_jobs"), p_class, h.get_class_proportion_jobs(i));
}
}
There is also something called get_random_passenger_class(), but that appears to be something else.
Assuming that something is wrong with the left window that needs to be looked into (either on your side or on my) what is the function of the get_class_proportions_sum()? It shows 100 on the right info window.
Are there any else variables I should/could use to show the information?
Have a nice trip in Switzerland!
Thank you for your work on this. I have recently returned from Switzerland and have been spending a lot of time post-processing all the pictures that I took there, so it will take me some time to catch up with Simutrans things.
Looking firstly at the specific issue that you report, what the screenshot appears to show is a situation in which there are more than 5 classes recorded in a building, which would be an error in this pakset. However, it is not one that I can reproduce. Can I check that you are using a pakset built with a version of makeobj built with the latest version of the code? Using an out of date pakset build might cause this as I have not implemented full backwards compatibility for earlier passenger-and-mail-classes builds in all cases.
It appears to work now when I updated the pakset, thanks!
However, most buildings now dont have any information about classes, but that is maybe intentional?
-----
So, I was thinking about how to present the information and I think there are these ways:
Either, you present the raw numbers, in a kind of similar way as the screenshoot above shows (that is, like the right window on the picture). This would present the data in the most accurate way as possible to the player, and the player then has to deschiffer the numbers by her/him self to understand what it will mean to your company.
The other way would be that I "translate" the information into more read-friendly text. This would mean that some accuracy would be lost in the translation, and the player would have to "trust" the text that would be written. I was thinking of something like:
"Commuters from this building travels mostly on ... "
"People from this building travels equally on ... "
"People from this building travels mostly on ... and ..."
.. or if there might be some shorter ways to say it. Then I would have to make some kind of algorithm with some limits that would guide the text.
A third way could be to calculate percentages and write something like:
"Very high class: 10%"
"High class: 20%"
"Medium class: 40%"
"Low class: 25%"
"Very low class: 5%"
I am leaning most towards the second version, sacrificing accuracy to readability, and abandone the percentage, as this way would create a single scentence explaining the information. Also, it should help minimize the confusion of what the information would mean, and it would add to the immersion that everything in the game is not just numbers and calculations.
If that wont work or is not desired, I would like to go with the percentages.
Splendid, thank you for testing. I have not yet added class data to most buildings - you can check which buildings ought to have them by looking in the .dat files. Adding these will take some time, as careful consideration and possibly research will be needed for each building, so this might take some time.
As to your question, my preference would be for the third solution that I suggest: I suspect that players generally prefer precise numbers, and the information in the second method seems to be excessively vague (and the thresholds are likely to be arbitrary). The third would also, I imagine, be easier to code, as well as quite easy to understand.
Thank you again for your work on this.
I have already made some tests to see what's possible and I find one bad thing with using percentages: rounding errors. I don't know how to deal with that, other than starting to use decimals. Adding the decimals together and put in some of the entries so the sum gets 100% could maybe also work, but that would be false information...?
I was giving it some more thoughts, and I am thinking more and more that the second way is better:
Remember the discussion we had a while ago regarding the way info window?
I talked about useful information and that I think good information is both deep and shallow.
I remember back in school when having a math class. We talked about how big distance there would be between the school and the city center. Somebody suggested a ruler on the map and then he precisely calculated that with two decimals. But then the teacher said it is unnecessary information to know the distance in km with two decimals, also even with any decimals, since, first, we can't determine exactly what is the city center, and second, going there would mean traveling several km's, and then a couple of extra meters wouldn't matter really.
Same as the measurement of the distance between London and New York: The rounding/number of decimals falls under the margin of error.
Translating this to the current discussion, I think that knowing the percentage, especially with decimals, is like the distance in the above example. Since it's a computer program, there would technically be no margin of error in the sense the example above described since the computer knows everything, but the player would not be able to use this information in any calculations, as the passengers showing up at a stop is the result of potentially hundreds of buildings, each with different class settings generating passengers with different destinations, time tolerances, etc etc etc making it very difficult for the player to calculate what will be showing up at a stop. Knowing the exact percentage of a given class of a building simply becomes too detailed information imho.
You can say that I am removing further decimals from the equation, "dumbing down" the information, but still telling the player what the tendencies is in the building. Would you not consider this?
I do not think that we need decimals (although I do not imagine that one decimal place would do any harm, either), but players do need to know about all the classes of passengers that a building will generate and their proportion relative to one another: it is not enough simply to say that passengers "mostly" come from one class, since that tells the player nothing about the other classes from which passengers come, nor whether "mostly" means 51% or 99%. In terms of readability, a percentage seems easier to interpret quickly than text about what classes that passengers" mostly" come from.
As to rounding errors, you might use a single decimal place, or alternatively, you might simply add code to round the percentage up rather than down when the equivalent decimal would be .5 or higher, which should hopefully lead to the percentages expressed without decimals equalling 100% when added together.
Ok, I will do the percentages. I think I have succeded making an algorithm that crudely will add "one" for every percentage that is missing. This will mean that the information might not be truly accurate but still accurate within 1%, but it would anyway not be, as the rounding would have to be done somewhere. It should not be noticed by the player, as 1% is not very much. Doing decimals would look silly I think. The way it works is, after calculation, it checks wether the percentages adds up to 100, and if not, it will cycle through the classes, adding 1% at a time to each class, unless the class is 0%. The exact same is done if there would be a result of more than 100%. My initial checks shows that this appears to work.
May I ask, what should be shown for default? I mean when nothing is specified in the datfile. If the default would mean that it would equally distribute classes, I could display that.
Also, when an entry is 0%, should it be shown at all?
Splendid, thank you very much for that. I would suggest that, where the .dat files specify no classes, each class should display an equal percentage (i.e. 20% each where there are 5 classes) so that the display is consistent between buildings.
Great! I will do that!
edit
Should be online now!
So, what information in the building view is now redundant? obviously you would still keep the population, jobs and the information currently benieth the class's part. What about the mail and passenger demand, are they redundant? How should mail be displayed by class?
I have made a new layouot for the building window without removing anything. I have only rearranged the info and made it a bit clever, for instance avoiding showing milions of entries of walking distance to stops.. It shows currently the closest six (now the closest is at top, not the furthest) and a new line will be displayed if there are even more stops. It will keep track of the furthest positioned stop and show the walking time and distance to it.
--
Going back to the class displayment in the halt window, I have finaly made a way for the passengers and mails to be sorted by classes. I have, however, done this in simhalt.cc, not in freight_list_sorter.cc as I originally wanted to. This means that it will be persistent and cannot be "sorted" to mix the classes. The reason to this, is because I have failed to see through the logic in the freight_list_sorter.cc.
Do you think it is sufficient, at least for the moment, if the passengers and mails are permanently sorted by class? Should the transit section also be sorted by class (which it currently is not), or maybe that doesnt matter..? Currently, I have left the class entries that I did make in the freight_list_sorter.cc, in case that would be needed later.
Now, going back to the vehicle class occupation, did you find a solution to the exploit?
The changes are online here: https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes)
Triple post in a row! ;D
So, finally I see the light at the end of the tunnel with the GUI for the passenger and mail classes.
What I have incorporated in the GUI:
* Class manager -> the tool to change the classes for the convoy
* Depot window -> passenger and mail classes are displayed. Also, the entire layout in the lower part is different as well as the upper right corner now displays some more detailed good info.
* Convoy details window -> Now shows loaded per class. (This is not finished, as James might change how passengers on reassigned classes behaves). Also, an upgrade notification has snuck in.
* Convoy info window -> Passengers and mails permanently sorted by accomodation.
* Halt info window -> Passengers and mails permanently sorted by class. Tranferring passengers and mail do not sort by classes.
* Building info window -> visitors and commuters percentage dependent on which class they prefer. Also manipulated the list of connected stops to show some usefull info as well as capping it if it gets too big. Sorted some other info in there as well.
TODO:
* In convoy details window, fix the displayment of passenger and mails.
* In convoy info window, assure that the passengers and mail in the convoy ALWAYS is traveling on the accomodations that is set by the player. Otherwise, GUI must change to also show passengers and mail traveling on non-existent classes.
* Consider wether the halt info window and the convoy info window should be able to be sorted differently. I would need to learn better skills to modify the gui_convoy_assembler.cc to be able to do that. If the current implementation is sufficient, thats great ;D
* Ensure that I am not wasting memory or doing something else bad, since I am creating a bunch of for() loops with comboboxes, vectors etc (I dont know how to check if it is efficient. It works as it currently stands thoug...)
* TRANSLATIONS! 8)
* More todos?
(https://farm5.staticflickr.com/4410/37380241231_2e5f5ea91c_h.jpg)
James, could you check this to see if there are any problems?
link: https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes)
Thank you for all your work on this. Apologies for not having had a chance to look into this yet: I have been busy outside the Simutrans universe, and on bug fixing when I do have Simutrans time. I will have a look at this when I have a chance. Thank you again.
No worries :)
Speaking of bugs, did you have a look on this issue:
http://forum.simutrans.com/index.php?topic=17401.msg165683#msg165683 (http://forum.simutrans.com/index.php?topic=17401.msg165683#msg165683)?
Because branch switching takes a little time, I am trying to fix all the significant master branch bugs first; however, I have noted that, and will look into that when I get a chance after I have merged the master branch and your latest work into the passenger-and-mail-classes branch.
Thank you very much for all your work on this, and apologies that I have not had a chance to look at this earlier. I have now merged and pushed your latest work onto the passenger-and-mail classes branch after merging from the master branch and fixing the bug to which you refer above.
I am in the process of reviewing this, so I may amend this post in due course, but one thing that I notice is that the passenger display always separates passengers by class. It seems to me that this really ought to be optional, as there are many situations when it is more useful to know the total number of passengers than the details of the class; a simple checkbox, perhaps, which, when unticked, reverts to the old behaviour of treating passengers from all classes alike.
Edit 1: I have made one or two minor fixes and changes to some GUI elements. One query, if I may: does the comfort display in the depot window take into account catering? The former comfort display used to show the comfort level with and without the effect of the catering on board a vehicle.
Edit 2: Similarly, it would be extremely useful to be able to see the combined passenger capacity in the deopt/replacer window for each vehicle that a player may purchase.
Quote from: jamespetts on October 20, 2017, 07:40:57 PM
Thank you very much for all your work on this, and apologies that I have not had a chance to look at this earlier. I have now merged and pushed your latest work onto the passenger-and-mail classes branch after merging from the master branch and fixing the bug to which you refer above.
Great! I will check up on that!
Quote
I am in the process of reviewing this, so I may amend this post in due course, but one thing that I notice is that the passenger display always separates passengers by class. It seems to me that this really ought to be optional, as there are many situations when it is more useful to know the total number of passengers than the details of the class; a simple checkbox, perhaps, which, when unticked, reverts to the old behaviour of treating passengers from all classes alike.
This was the trade off from incorporating it directly in the simconvoy_t and simhalt_t instead of in freight_list_sorter_t. A better way overall would be to have it in the freightlistsorter, changing the sort button to a scrolled list and add a bunch of new sort options, but I had to give up on that since I could not figure out all required steps in the freightlistsorter. I think some remains of my attempts are commented out in that file.
Therefore, I initially did not want to do too much work in the sim..-files. I do, however have an idea that should be quite easy to implement that would achieve what you want.
Quote
Edit 1: I have made one or two minor fixes and changes to some GUI elements. One query, if I may: does the comfort display in the depot window take into account catering? The former comfort display used to show the comfort level with and without the effect of the catering on board a vehicle.
I have made the comfort display in the depot window (and elsewhere) follow this syntax:
"Base comfort + catering comfort"
So if the catering car has a comfort of 100 and is a catering car, it would look like:
"100 + 20"
I like to know the logic behind the comfort, and I think doing it this way helps the player see through the mechanism.
What do you think?
Note that the "+20" only comes visible if the cars class matches its catering level.
Quote
Edit 2: Similarly, it would be extremely useful to be able to see the combined passenger capacity in the deopt/replacer window for each vehicle that a player may purchase.
So you mean in the depot window something like:
P_class[0]: 100
P_class[1]: 50
Total: 150
That would actually also solve the overcrowded display issue.
Quote from: Ves on October 21, 2017, 09:17:23 AM
This was the trade off from incorporating it directly in the simconvoy_t and simhalt_t instead of in freight_list_sorter_t. A better way overall would be to have it in the freightlistsorter, changing the sort button to a scrolled list and add a bunch of new sort options, but I had to give up on that since I could not figure out all required steps in the freightlistsorter. I think some remains of my attempts are commented out in that file.
Therefore, I initially did not want to do too much work in the sim..-files. I do, however have an idea that should be quite easy to implement that would achieve what you want.
Excellent - do let me know how you get on or if you have any queries.
QuoteI have made the comfort display in the depot window (and elsewhere) follow this syntax:
"Base comfort + catering comfort"
So if the catering car has a comfort of 100 and is a catering car, it would look like:
"100 + 20"
I like to know the logic behind the comfort, and I think doing it this way helps the player see through the mechanism.
What do you think?
Note that the "+20" only comes visible if the cars class matches its catering level.
This is good except for one issue: the catering level used by each passenger depends on the class of passenger, not the class of accommodation. A wealthier passenger does not buy a cheap meal simply because he/she has decided to travel in a lower class of accommodation. We do need to show the comfort modification for the catering irrespective of the class of accommodation.
Quote
So you mean in the depot window something like:
P_class[0]: 100
P_class[1]: 50
Total: 150
That would actually also solve the overcrowded display issue.
Yes, indeed, that would work well.
Quote from: jamespetts on October 21, 2017, 08:42:45 PM
This is good except for one issue: the catering level used by each passenger depends on the class of passenger, not the class of accommodation. A wealthier passenger does not buy a cheap meal simply because he/she has decided to travel in a lower class of accommodation. We do need to show the comfort modification for the catering irrespective of the class of accommodation.
Hmm, I dont really understand. Are saying that a p_class[4] passenger riding in a p_class[2] accommodation in a train featuring a catering car level 2 would not get the bonus comfort?
Or do you mean that a p_class[4] passenger riding in a p_class[2] accommodation in a train featuring a catering car level 4
would go and eat in that car, even though he has purchased a p_class[2] ticket?
I did write it a bit clumpsy however and it might just be a misunderstanding:
The +20 will come to all classes
at or above the catering level. Should it do something else?
Quote
Yes, indeed, that would work well.
Great! It will be located above the class sections if there are two classes or more!
edit:
clarification
Actually, the comfort modifier of catering is currently class insensitive; however, yes, a class 4 passenger would buy a class 4 meal even if travelling in a class 2 compartment - but this only affects catering revenue, not the comfort modification.
Ok, so how precisely does the catering affect the comfort in a convoy? I think Im still a bit confused :P
But I think I then will rephrase the "catering_income_pr_km_(when_full)" to something like "estimated_catering_income_pr_km" to illustrate that it is difficult to predict.
I have now made a button, both in the convoy window and in the halt window.
I do however have one problem:
I wanted the window to remember the state of the checkbox, so you dont have to tick it every time you open a window. I have made what I think is correct, however the state is not saved!
Even so, in the convoy window, the state randomly alternates between checked and unchecked upon opening convoy windows...
suggestions?
the changes are live on my usual github branch (dont touch the translation basefile yet please! :) )
The catering affects the comfort very simply: there is just an increase of a fixed amount in the comfort of the whole convoy when one or more catering vehicles of a given level is added to the convoy. This is separate from the catering revenue.
As to making the state saved, may I suggest using a static variable?
Edit: I have now had a chance to review your latest changes: thank you for that. One bug that I have noticed is that, in the vehicle classes display, when a vehicle has multiple classes, the wrong capacities appear to be shown: if you use the testing saved game that you sent to me and reset the classes on the passenger trains, you will see that the displayed class capacity for each class is only 48 when the class separation display is enabled, whereas it shows it correctly as being >400 when it is disabled.
Another issue with the vehicle class display is that, while it shows something different from the stop classes display (i.e., what class of accommodation that the passengers occupy rather than their underlying class), the display appears to be exactly the same, which is likely to cause players to think that something is wrong when some passengers marked in a stop as "medium" board a vehicle and immediately become "low". Do you think that you could make it clear that different things are being represented here?
Also, I notice that the new checkbox takes away some vertical space from the convoy and halt information windows; could this not be positioned so that it does not do this? The current position reduces the useable window space.
In the depot window, the total capacity display is very useful, but I notice that it is only used when a vehicle has more than one class. It would be much easier to interact with, I think, if it were consistent, and displayed the total capacity even when a vehicle had only one class so that players would have only to look for a single total capacity display for every vehicle. Also, is there any need to have a new translation text, "total_capacity" for this? Would not the existing "Capacity" text be better, to take away the need for this text to be re-translated into a large number of languages?
Thank you again for your work on this - it is much appreciated.
I must say that the
freight_list_sorter_t::sort_freight(pass_fracht, buf, (freight_list_sorter_t::sort_mode_t)freight_info_order, &pass_capacity, "loaded");
.. is really neat and simple, but oh my god so unflexible! :redx:
"pass_fracht" will mix together all good of the same type, all being passengers = all classes are mixed together resulting in just one block of all passengers.
So, solution seems to be to have one "freight_list_sorter_t::sort_freight( .... )" per class, which works nicely.
Then we also can have different "loaded" text for each class, namely what class the block of passengers belong to.
BUT!!!
the "&pass_capacity" gets totally confused about this and wonders what is going on! the "&pass_capacity" have been told that there are, say two classes of passengers with xx amount in each class, and then it is told to display twice, which now results in four accommodation entries!
So it seems to be a catch 22 here, or am I missing something? I know I really only have very basic programming skills and it has been quite hard for me to sort out how that "&pass_capacity" works, and I think Im not far off in my description.
Trying to do a similar "&loaded" containing different classes text strings, but I dont know enough about it to make it work, and it seems that the ::sort_freight(..) doesnt accept such a value either. And then there would still be the issue with all passengers mixing up...
So I think I will have to dig deaper into the freight_list_sorter anyway ..... :::)
-----
Regarding some of the other issues, I have modified the depot window as you suggested and with the old translations as well.
I had initially also fixed the different display in the vehicles info window, to specify that it was in an accommodation of a certain class, but that had to be revised due to above rant ;D
The issue with the wrong amount displayed in the vehicle window has also its roots in the above rant and is "fixed" in the sence that the correct numbers is sent to the freight_list_sorter now!
With regards to the catering vehicle, I am not sure how it would be best displayed, but I must say that I lean towards the current implementation, and maybe an additional text in paranthes below the comfort rating in the class manager window displaying when a catering car is present: "(p_class[X] or higher passengers in this accommodation +XX)".
This text would only be visible in accommodations lower than the catering level.
What happens if you put two catering cars of different catering level in the same convoy?
Im sorry about the rant part! Obviously it is designed this way with a purpose and it really only is my own lack of programming skills that creates my frustration. So no hard feelings! ;D
edit:
Ok, so I have taken the first step on this, and this was, I think, to make the sort buttons in the vehicle and halt info windows into comboboxes. As I am highly likely about to add more sort options, I really think that the push button is making the player loosing the overview (already, I tend to loose the overview and constantly overshoot the sort option I want). I hope that you would agree that this is a good idea :)
Now it look like this, and I believe it works as it is supposed:
(https://farm5.staticflickr.com/4496/37860799436_b2f5030e1f_b_d.jpg)
I think that I will remove the classes checkbox again, and incorporate it in the sort options. If I can make it work, we could have a "sort by class amount" and a "sort by class via". Other suggestions?
Other sort options I had in mind could be "sort by commuting/visiting" but first I need to get things running!
The changes, incl the fixes mentioned earlier, are on my github: https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/gui-classes)
Splendid, thank you for your work on that. One comment at present is that the station combobox is too narrow to fit in the phrase, "destination (detail)" without clipping to the right, so this probably needs widening somewhat.
There does seem to be sense in reducing the number of separate buttons/controls and making the sorting option set into a combobox given the number of options that are now necessary, and your ideas for further options are interesting, too (subject to making sure that there is enough horizontal space to display the text).
---
In relation to the &pass_capacity, I have not had a chance to examine this issue in detail, but I wonder whether you understand quite how passing parameters in this way works? A confusion as to this might be behind the issue.
Let's start from the beginning. Suppose that we have a variable "kittens". We define it thus:
uint32 kittens = 5;
This is very straightforward. If we pass this variable to a function in the normal way:
cuddle(kittens);
then we pass a copy of the variable to the function. The original variable is not changed.
We can alternatively take a pointer of "kittens". This is a variable containing the address in memory of another variable. So, we can have:
uint32 kittens = 5;
uint32 kittens_pointer = &kittens;
cuddle_pointer(kittens_pointer);
Note the following:
cuddle_pointer(&kittens) does the same here as cuddle_pointer(kittens_pointer) and the function signature of "cuddle_pointer" would have to accept a pointer. Thus, for the first example, it would be:
uint32 cuddle(uint32 cuddly_thing)
whereas in the latter it would be
uint32 cuddle_pointer(uint32* cuddly_thing)
In the function itself, to get to the thing being pointed at (in this case, the number 5), we have to dereference the pointer. So, for "cuddle" we could simply say:
uint32 cuddle(uint32 cuddly_thing)
{
return cuddly_thing + 10;
}
but for "cuddle_pointer", we would have to provide:
uint32 cuddle_pointer(uint32* cuddly_thing)
{
return *cuddly_thing + 10;
}
In both of these cases so far, the function of "cuddle"/"cuddle_pointer" is identical, and no difference is made when we use the pointer. However, when we pass a pointer (otherwise known as passing the variable by reference, any changes that we make to the variable in the function to which it is passed are permanent. Thus, suppose that we have the following code:
uint32 cuddle(uint32 cuddly_thing)
{
cuddly_thing += 10;
return cuddly_thing;
}
uint32 cuddle_pointer(uint32* cuddly_thing)
{
*cuddly_thing += 10;
return *cuddly_thing;
}
uint32 kittens = 5;
uint32 result = cuddle(kittens);
cout << kittens;
cout << result;
uint32 pointer_result = cuddle_pointer(&kittens);
cout << kittens;
cout << result"
the output when running this code would be:
5
15
15
15
Indeed, in the case of the cuddle_pointer function, one could simply omit the return value and provide:
void cuddle_pointer(uint32* cuddly_thing)
{
cuddly_thing += 10;
}
void cuddle(uint32 cuddly_thing)
{
cuddly_thing += 10;
}
uint32 kittens = 5;
cuddle(kittens)
cout << kittens;
cuddle_pointer(&kittens);
cout<<kittens;
which would give the result
5
15
I suspect that what is happening with pass_capacity is that this is being overwritten multiple times before the output on screen is generated.
---
In relation to catering vehicles, where two catering vehicles of different levels are in the same convoy, the catering vehicle of a lower level is ignored for the purposes of comfort and catering revenue. Really, the only change needed to your implementation here is to show the catering level's effect on comfort in the way that you already do irrespective of the class of accommodation that that vehicle carries (and making sure, if you have not done this already, that this applies to the maximum comfortable journey time as well as the numerical display of comfort.
---
Thank you again for all of your ongoing work on this - it is much appreciated.
Thank you for your detailed explanation. I have read it a couple of times and will need to read it again a bunch of times to better understand how it works and how it can be used.
Yes, the combobox in the halt info is too narrow, and then I had already widened it a bit from original! I will widen it even further, perhaps as wide as in the vehicle info. That depends on how the other buttons behave when window is resized.
Splendid, thank you. Do let me know if you have any more queries about the coding principles to which I referred.
So, now it is starting to get somewhere:
I have created two new sort modes in freight_list_sorter, by_class_detail and by_class_via. They are essential identical as their normal counterpart, except they are permanently sorted by class. The first sort option looks very much like the sort layout that the version 1 looked like.
This is now online on my github branch and I think there are no bugs in that part.
However, that was the halt window, the convoy window is way more complicated by some reason. First, the same exact culprit-parameter, the &capacity, apparently makes some funnynes together with what code I have written. It makes the class sections in the good window melt together, but do only show a max capacity of the topmost class. Replacing &capacity with a "NULL" in the simconvoy fixes all issues, however, no capacities is written at all.
Which brings me to the real major issue:
Wen a class sorting mode is selected, we want it to sort by accommodations, and not classes. This was easy in simconvoy, as we can count the classes there and fix all issues with that.
In freight_list_sorter, there doesnt really seem to be a way to check where the good is. In fact the only way to check that is to see if a &capacity value is given, and if there is, the cargo is in a convoy of some sort. There is no way to see which classes the convoy has, and even less which passengers are sitting in which classes! I have not found anything in the simware_t either.
Therefore, I would REALLY be happy if there was some way to tell where a package of ware_t is. So... is there?
Thank you for that. I have noticed an issue with the translation, however: I have added translations for the class names to the en.tab for Pak128.Britain-Ex in the passenger-and-mail-classes branch (you might want to fetch the latest version of this and compile it for your testing purposes). However, the translation for the class names do not work in the halt list when they are shown after the details as to whether the passengers are visiting or commuting - instead, the untranslated text is shown. I suspect that this is because you are not translating the exact same string, perhaps translating something that contains a format specifier, instead of using the format specifier to produce a finalised string that is identical to the translated string, and then translating it.
In terms of telling where a ware_t packet is, the only way of doing this is to sort through the vehicles themselves and check which ware_t packets are stored in which arrays. It would take too much computational effort and memory bandwidth to pre-cache this information in every single ware_t packet in the game just for a GUI feature.
Thank you again for your work - it is much appreciated.
Quote from: jamespetts on October 27, 2017, 09:58:21 PM
Thank you for that. I have noticed an issue with the translation, however: I have added translations for the class names to the en.tab for Pak128.Britain-Ex in the passenger-and-mail-classes branch (you might want to fetch the latest version of this and compile it for your testing purposes). However, the translation for the class names do not work in the halt list when they are shown after the details as to whether the passengers are visiting or commuting - instead, the untranslated text is shown. I suspect that this is because you are not translating the exact same string, perhaps translating something that contains a format specifier, instead of using the format specifier to produce a finalised string that is identical to the translated string, and then translating it.
In terms of telling where a ware_t packet is, the only way of doing this is to sort through the vehicles themselves and check which ware_t packets are stored in which arrays. It would take too much computational effort and memory bandwidth to pre-cache this information in every single ware_t packet in the game just for a GUI feature.
Thank you again for your work - it is much appreciated.
Hmm, that is rather odd as I have tried to make sure that all translations of the classes are done before any formatting to them and cant initially find anything strange with this example. I will check it up though and with a new pakset!
Ok, that is a valid point. I had a thought about adding a new parameter to the sort_freight(....), namely an array of *something* that I could use in the freight_list_sorter. The risk of doing something stupid, if at all working, however, I feel is quite big, so I am hesitating a big bunch before doing that.
Do you have any suggestions as to how to store a list of classes to go along with the good, which I then can pull out in the sorter? Is now a good time to get friendly with the
& -sign?
editfound the translation issue: I translated it one step too early, before i had combined p_class[%u] and the number to replace the %u. Should be solved now!
Wow, after been staring and swearing so much to the untranslated texts, it looks really strange to see something else there now! :P
One suggestion if I may: I think it is very confusing that you have only translated it to "high" "very high" etc. "high-what??" the player will ask. Would it not be a better idea to call it "high class" "very high class", "low class" or in some other way incorporate the word "class" into your translation? The way I have constructed the gui, somehow require the names of the classes to be a little bit self explanatory. Otherwise the translation system would be very unflexible for other languages.
Thank you very much for your fix of this issue: that is much appreciated. I will test this when I have a moment (I am rather preoccupied with other things this week-end).
As to the & sign, there is never a bad time to learn about that: you will need to learn first about pointers generally and then about the special case of references in C++ (which is a somewhat cosmetic modification of pointer syntax, the underlying logic being the same).
In relation to "high", "medium" and "low", etc., these terms were intended to be made clear by their context: "high class" would not be clear on its own, as, in English, that would tend to imply something of fine distinction rather than just people who can afford to travel in Pullman carriages (and, similarly, "low class" would suggest something degenerate, not just people who can only afford to travel by 'bus if at all). "Class: low" or "Class: high" (etc.) would not have these connotations, as they come with the specific stock phrases. The actual translations of the class names should not, however, have to signify by themselves that they are names specifically of classes rather than of anything else.
I have now had a chance to test this; I found a few bugs which I have now fixed, and I have also made clearer the textual description explaining which classes that passengers/mail are travelling in aboard vehicles.
Sorry for my absence lately, I have had quite alot to do!
I saw your commit on the github, the things are, however, not yet really finish so I dont think there is much point in trying to correct stuff just yet, especially not in the convoy-window :)
I see you rearranged some things in the freight_list_sorter as well, and although your solution looks neater than my original implementation, some functionallity seems to be lost, namely that the classes in the detailed good view dont need to be shown if the good is already sorted by class.
Regarding the translation of the classes, what if you translate the classes like this:
p_class[0]
Class: Low
.. then you get no confusion what is "low", and we dont enforce other translations to have to translate a "Class:" every time it is shown, like in the detailed good view described above?
Ahh, I did not realise that you had not finished: my apologies. Do feel free to re-add any useful feature that you think that I have accidentally removed, provided that it works.
As to the class translations, surely it is more flexible and less redundant to have the names of the classes translated separately from the word describing what sort of thing that they are naming?
I will try to explain with an example.
Look at how you have translated the way constraints:
Quote from: en.tab
#__________________________________Way constraints__________________________________
#__________________________________Way constraints__________________________________
# Permissive
Permissive 0
DC Third Rail Electrification
Permissive 1
DC Overhead Catenary
Permissive 2
AC Overhead Catenary
Permissive 3
DC Fourth Rail Electrification
Permissive 4
Waterway
# For the DLR - reserved for future use.
Permissive 5
Light rail electrification
Permissive 6
[Unused] Permissive 6
Permissive 7
[Unused] Permissive 7
# Prohibitive
Prohibitive 0
Tramway
Prohibitive 1
Tube tunnel
Prohibitive 2
Barge canal/small river
Prohibitive 3
Tub boat canal
Prohibitive 4
Narrow canal
Prohibitive 5
Ship canal/medium river
Prohibitive 6
Large ship canal/large river
Prohibitive 7
[Unused] Prohibitive 7
Here, you see that the name of the constraints are self explanatory. This means that, if just ever this text appears on the screen we know what *this* is about. So to explain what constraints a vehicle can travel with, you only need this:
May use:
Ship canal/medium river
If, say we play with renaming the constraint of some of the "prohibitive" -rivers to:
QuoteProhibitive 2
Small
Prohibitive 4
Narrow
Prohibitive 5
Medium
Prohibitive 6
Large
... the information would, principally, be the same, but you would need other elements of the GUI just to explain *what* this is. The current translations of the classes makes these extra gui-elements required, because otherwise the classes entries in the gui makes no sense in for the player.
Following the example, we would get a text like this in the depot window:
May use:
Medium
This is why I am concerned with how the classes are translated and what GUI-elements it would enforce.
For pak.sweden, I was considering something like this:
Lyxklass (Luxuary class)
Finklass ("fine" class (dont know if there is a direct translation other than that))
Almänklass ("public"(or similar) class)
etc...
This does get linguistically a little complex, as there is no direct equivalent. The way constraints, importantly, do not say "Constraint: tramway": they just say "Tramway".
The complexity is that, in English at least, the words for describing classes of accommodation ("first class", "third class", "steerage", "tourist class", "business class", etc.) are not the same terms as apply to the description of the people who can afford to travel in each class, for which there are no standard terms. The only way of describing classes is to have a word indicating "class" somewhere and then the names of the individual classes following. It would be very redundant to have the word "class" repeated for each description of each class (and there is no such repetition with the constraints).
QuoteThis does get linguistically a little complex, as there is no direct equivalent. The way constraints, importantly, do not say "Constraint: tramway": they just say "Tramway".
If I do not misunderstand what you write, this example exactly illustrates what I mean! ;D
We can indeed say "tramway", and everybody will understand what we are talking about, however the same cannot be said about "low". At least I would not have any clue that it would refer to classes, if I did not read it anywhere else.
I think somewhere that what I clogg myself up about is, that "low" really is only an adjective. Per definition, it requires another word to give meaning at all. In addition, almost everything can be low: people, rates, chairs, amounts, prices, houses, humor etcetc. As it stands in the good view section in the cargomanifest of stations and vehicles, it is just a simply "low", like this:
8 passengers > Railway depot (electric) <228, 24> (Tumpbourgh, commuting, Very low)
So what is very low? their mood? their motivation? their satisfaction? their transfer rate?
It would even be better to use the word "cheap", since its not that many things that are called cheap, like "low", and people generally think of the word cheap in an economic way (I guess), so the step to think of a class to be cheap would not be that far away:
8 passengers > Railway depot (electric) <228, 24> (Tumpbourgh, commuting, Very cheap)
Well, the simple solution seems to simply add a "class:" from the GUI-side everywhere these misunderstandings might happen. But designing a flexible GUI around adjective entries is different than designing GUI around other kinds of entries, like the constraints. Forcing a "class:" down on every language can be very limiting for them and will also feel inconsistent when some parameters have a descriptor and some do not. What comes naturally in english might be quite arbitrary in other languages.
Even though the sentence: "Class: Very low" would not change in size if the "Class: " -part is in the GUI or in the class name, it can really be redundant and take up space in the window in other languages. like in Danish: "Klasse: Luksusklasse" ("Class: Luxuary class") which could have been written much more efficiently omitting "Klasse: ".
To use some other words:
Your translation are telling the rates of the classes, instead of the names of the classes.
But I do understand that it seems to not work grammatically with, say "low class". Also I understand why you want to use the vocabulary "low" "medium" "very high" as it is kind of universal and would apply for all ages. Names like "tourist class" would not look well in the 1800, and "pesant class" would not look good today. Suggestion for changing names dependent on era incoming? ;D
However, some words, like luxuary class, I do not really understand why it could not be called. If a passenger is a luxuary class passenger, he/she would prefer to travel on the luxuary class at any time? Simply put, the passengers p_class[ x ] value is only their prefered method of travel?
But what do you suggest we do then?
The ultimate problem is that there is not a good, simple vocabulary to describe what we want to describe. We either have to specify something like:
Class:
15 Very low
12 Low
25 Medium
1 High
0 Very high
or something like
15 Class: Very low
12 Class: Low
25 Class: Medium
1 Class: High
0 Class: Very high
The latter is somewhat redundant, although I note that the former might cause difficulties for other languages. Bear in mind that "luxury class" does not really make sense in gameplay terms, as it is quite possible for a very low comfort vehicle to be assigned to a high class (Concorde, for example, was cramped compared to even economy class on contemporary airliners, and early aircraft such as the Ford Trimotor were really very uncomfortable compared to luxury carriages on railways, but wealthy passengers used them because of their great speed advantage).
One possible modification is to use the word "Wealth" when describing passengers and "Class" when describing different accommodation in vehicles, which might at least alleviate the confusion between the use of these terms in the vehicle and stop windows. This would, of course, necessitate the splitting of the terms that describe each class/wealth level from the terms that describe whether it is a class or wealth level, to give this sort of result:
Wealth:
15 Very low
12 Low
25 Medium
1 High
0 Very high
and
Class:
15 Very low
12 Low
25 Medium
1 High
0 Very high
with the "Wealth" and "Class" translation texts being distinct from the "Low", "Medium", etc. translation texts for the descriptions. How well would this translate to Swedish, do you think?
The first example you give is obviously much better looking than the second example, so I understand the reason not to use "class: very low" as the translation.
Quotewith the "Wealth" and "Class" translation texts being distinct from the "Low", "Medium", etc. translation texts for the descriptions. How well would this translate to Swedish, do you think?
Do you mean that you would just call it classes and wealth instead of just only classes, but the actual class names are still "Low", "medium"?
I dont think that changes anything, since you would still end up with "low"-
what? in certain gui-elements.
My example changed the name from "low" to "cheap". The list of names could be something like this:
Very cheap
Cheap
Economic
Expensive
Very Expensive
Admittedly, translating it to such, although it might help the player understand what he looks at more than "Very low", "Low" etc, might as well confuse the player into other misunderstandings about the class system.
We could do a compromize, where we make two sets of translations: One set is for all places where there is no doubt what it relates to, and the other set needs a more fullfilled name (ie. "Class: Low") to places where confusion might arize? So each class would have this:
p_class[ x ] (short version) and
p_class[ x ]_full_name (long version).
With this, you would be able keep the "Low", "High" and other languages could do that as well, or they could have more like real class names, making any clarifications unnecesary for them.
Only downside is that it gets more complicated to maintain, as well as more translator-work...
The idea is that the GUI would always indicate, using the words "Class" or "Wealth" (which would be separate, specific translation texts) to what the names "low, "very high", etc. are referring. "Cheap" and "expensive" makes no sense, as this would not meaningfully refer to the passengers themselves rather than the accommodation (and in any event, it would be more elegantly expressed as "Price: Very low", "Price: High"). A system in which the reference to the kind of thing that is low or high (etc.) is part of the very same text as the low or high itself, even in only some places, will always result in the sort of inelegant duplication seen in the second example in my previous post.
Incidentally, I have noticed an issue with the UI for buildings: in all town buildings, there is a display of "visitors_from_this_building" and "commuters_from_this_building". However, this does not correctly represent the position.
For residential buildings, there needs to be a single display showing the same data as for "visitors_from_this_building" currently. This should be labelled "Passenger wealth", "wealth of passengers residing here" or similar.
For non-residential buildings, the existing two displays need to be retained, but the labels need to be amended, as "visitors/commuters from" the building makes no sense for destination buildings. "Wealth of visiting/commuting passengers demanded" or similar would be a better description.
For industries, there is currently no information given about class proportions, which is where it might be especially helpful for players, particularly given the importance of the logistics feature.
Thank you again for your work on this.
Are you sure you want to use the word "wealth" for passengers? Will it not be confusing when passengers is sorted by "wealth", but the vehicles has "classes"?
Anyway, I will take a look at the issues!
In the industries window, I have to remake parts of the window in order to show the classes, I will start on that as soon as possible!
In order to save space in the building window, what about a syntax like this:
wealth of visitors (commuters):
20% (10%) Very low
30% (45%) Low
35% (30%) Medium
etcetc...
or:
wealth of visitors/commuters:
20%/10% Very low
30%/45% Low
35%/30% Medium
etcetc...
.. that would help alot in the industries window!
edit:
Maybe also make the window a bit more intelligent:
If "Visitor demand: 0", we dont need to show visitors classes
If "Jobs: 0" (that is, there is no jobs in this building), we dont need to show commuters classes
makes sense?
The idea is that it will be less confusing, as a passenger of one level of wealth might board a different class in some circumstances, so it would be very confusing if they were represented to be identical, as it is important for players to be able to distinguish between these two importantly distinct concepts easily.
I do not think that "wealth of visitors" is a good phrase here, as this implies that what is being represented is the wealth level of the passengers who have in fact visited in the past: "wealth of visiting passengers demanded" avoids that problem by describing what is being displayed accurately. Certainly, it would make sense not to repeat the phrase and to have
"10% Very low
20% Low
20% Medium..." (etc.)
after that designation. Were you asking more about compacting visitors/commuters? The first example seems to me to be apt to confuse (it is not clear what being in brackets designates). The second is slightly better, but it is still not absolutely clear that what is being referred to in each line is, in turn, the wealth of visiting passengers demanded then the wealth of commuting passengers demanded. I think that it would be clearer just to have two separate lists where applicable, although I definitely agree that, where there is no visitor or commuter demand, the respective section should be omitted.
While it might be more obvious with separate lists, the information will take up double amount of space. In some windows, there might already be a very long list of other things (industry windows shows consumers, suppliers and list of goods), so having a 13 row long section of class information will be heavy in the window. I think the second one of my suggestions will work just fine, and then translate the description to:
"Wealth of demanded visitors / commuters:"
This is even more important if new features requires even more information in the windows.
I see the point about compactness of windows. I suppose that, ideally, one would have a tabulated display with columns, but I imagine that that would be much more difficult to write?
I have putted together a variant looking like this:
(https://farm5.staticflickr.com/4528/37559909474_c0211a197d_h_d.jpg)
I could not find a building with only visitors and no workers, which is also not a residental house.
Suggestion:
Renaming "passengers" to "residents" in the RES-building?
Question:
What does the "Passenger success this year (visiting)" actualy mean? Is it residents of this building finding a destination? or is it a combination of residents of this building and visitors to this building?
I get confused when the same parameter (say, the get_passenger_success_percent_last_year_visiting()) returns a percentage of 100%, while the "passenger_success_percent_last_year_visiting" returns a number.
Personally I would like it to stay plain numbers and no percentages.
Lastly, what appears to be a bug:
In the "Sawmill" window, the "commuters this year" returns a strange number. The code is duplicated from the buildings window.
Splendid, thank you for your work on that. In respect of the industry commuters: these are processed differently for industries, and players need to refer to the graphs to see the number of commuters, so the "Commuters this year" line needs to be removed.
To answer your question, "Passenger success this year (visiting)" refers to the proportion of visiting passengers generated by the building that have successfully completed their journeys this year (and the same applies to the equivalent for commuting passengers and the previous year). It is only relevant to passengers generated by that building, and is not affected by passengers visiting that building.
I agree with your suggestion of renaming "passengers" to "residents".
Thank you again - this is all most helpful.
Hmm, then I think the numbers doesnt match somehow:
Visitor demand: 2
Jobs: 126
.. so you would expect these figures to somehow show up in the charts. However:
The passenger demand diagram says: 12, and have been doing that for alot of months.
So something is not scaling properly.
Also, it doesnt say in the charts wether we talk commuters or visitors.
Technically, commuters (filling the job slot) are the ones that should fix the boosts, however, visitors should also do that for some industries, like shops etc.
How is all this related, or is it not really related yet?
I would be really happy if we could display the "commuters this year" benieth visitors (as in the screenshoot), since this would give the player a very good overview instead of looking in the charts.
Is this not possible at all?
edit:
Also, not to forget, the player is used to be able to see "commuters this year" in other buildings.
The factory system is quite complex, since the boost system introduced in Standard some time ago does not fit well with the passenger generation system introduced in Extended in circa 2013; however, plans to reform the boost system radically will need to wait before being implemented, as this would be a huge amount of work.
I did spend quite a considerable amount of time recently looking into the calibration of industry passenger demand and fixing quite a number of bugs, but this was a few months ago now, and I can no longer remember all of the details. You can always look back at the Git commits a few months ago to find what I modified as far as industry passenger demand was concerned.
If you really want to have a "Commuters this year" display, you could always just take the information from the graphs (or, rather, from the data that comprise the graphs).
Have successfully compiled the very latest program and Britain Ex pakset in this branch. Everything seems to be functional, although the convoy manifest list is always empty, regardless of whether it's passengers, mail, or goods being carried. I have carefully copied all the *.tab and *.txt into the directory where I run the game, but cannot find what might be missing. Any suggestions?
(http://wlindley.com/simutrans/missing-cargo-manifest.png)
Glad that you have tried it! That manifest is still work in progress. I have to find a way to sort the passengers by accommodation, which appears to be more difficult than it might seem. The "show class" button will disappear and be replaced by sort options for classes.
So for the moment, no good is shown because of that.
Thank you for your feedback.
I should note that I am hoping to have this integrated with the master branch quite soon, preferably by December, so that more rigorous testing can start (including starting online servers) and so that the amount of work involved in maintaining two separate actively developed branches comes to an end. This does not mean that all work needs to have been completed on this by December, but we should aim at least to have it playable by then. As far as the GUI is concerned, do you think that you would be able to make what we have workable and fix obvious bugs (such as the failure to display passengers and goods, as here) by early December, even if all the desired UI features cannot be completed by then and even if this means temporarily removing some of the more advanced UI features that are not essential for the passenger and mail classes features to work?
The thing being, that version was the version which had the bug of not showing correct amount of good in the accommodations, which we talked about earlier. What I try to achieve now is in fact to get these basic stuff to show up as desired.
If you are getting stuck with the task of fixing this bug, might I try to assist? Have you made any more progress since your last update of code to Github from which I could start in attempting at least a basic solution to this for the time being?
Thank you for your work on this so far.
I have not had much time lately, I don't recall currently what you have got and what you haven't. I'm not home tonight so I can't check, but I should be able tomorrow to check and update my github.
Splendid, that is very helpful, thank you.
I have merged your latest code with my gui-classes and uploaded it to github. I dont recall what you had incorporated and what not from beforehand.
AND, please save the base-translation file. I do still fill it out with new entries! :)
edit:
I have now spent a few hours today trying to figure some of these things out, and somethings have been solved and others have arized.
For instance, I can sort passengers (only passengers, it seems) by classes in the convoys now, but I cannot with mail.
I do, however, realize that I have made a big error in simconvoy that I need to fix, but I am too tired to do that now Im afraid :P
It is on Github now!
I think that I have fixed what appeared to be the principal bugs, although it is not as clear as it might have been what the intention was in many cases. Ves - can you let me know what parts that you think are and are not working as at my latest fixes in case I have missed anything important?
Edit: Having noticed your edit, after my original post, I see that our possible solutions may have collided. I will have a look at what you have managed and see what we come up with.
Incidentally, can you upload a saved game in which the specific problems that you report can be reproduced reliably?
Edit 2: I am afraid that there are some major merge conflicts between our respective fixes, and I am not entirely sure what you had intended. Can you check whether things are working better in my version or your version so that we can consider how to deal with these difficult merge conflicts?
Edit 3: Incidentally, we also need to show the class proportions, etc. for depots and stations.
Oh no, too bad with conflicts! :p
I'm away for the day, but maybe I can look into it tonight. We really should coordinate so we don't do duplicate work! :)
Splendid, please do. I did the work that I did because I thought that you had got stuck with some of these things and wanted a little assistance, so I tried to fix some of the bugs; my apologies if this was not clear. Thank you again for your assistance with this.
No worries, I appreciate all the help I can get! :)
Writing in a new message, so as to minimize confusion this time ;)
I have now merged it with my work, but I do not know if I have merged it correctly. The display looks odd with quite some bugs somewhere, and I can see some parts of code you have added which looks like they are supposed to be used by simconvoy.
Trying "passenger-and-mail-classes" in the state you left it, it looks luckily much better, as my code is not interrupting it. However some issues:
* The max capacity of the classes are wrong, they show the total amount of passengers in the convoy, not the total for the classes.
* The passengers is now always sorted by class.
Some questions:
you have modified the void freight_list_sorter_t::sort_freight(vector_tpl ...... ) and added uint8 g_class to the end. I can see that it is used a few times in the code, but I had imagined that it would also be used in conjuction with simconvoi_t?
I dont understand why you have modified the sort section with parameters like (g_class == all_classes || wlist[ i ].get_class() == g_class) when sorting by anything else than by class/wealth?
Lastly, should I start digging in the code, or would you like to? ;D
I am currently away from home, so cannot easily answer in detail. However, can I clarify whether the issues that you report are before or after your merging work?
I think our two versions do not work well together at all, so I might have to start on a new branch to continue working :P
The issues I describe comes from the unmodified "passenger-and-mail-classes" branch, so that will say without any merging.
Also, another odd bug I have come acros now is that mail appear to exist also in 5 classes.
Using the savegame linked below , you will notice that we suddently now have "m_class[ 4 ]" class mail. This you can also see with the version compiled directly from passenger-and-mail-classes branch.
savegame:
http://simutrans-germany.com/files/upload/classes_GUI_vehicle_tests_6.sve (http://simutrans-germany.com/files/upload/classes_GUI_vehicle_tests_6.sve)
Also, could you help me understand the changes you have made?
edit:
Added savegame!
Splendid, thank you for the report. I think that I have fixed the mail issue, which was a problem in the passenger generation code whereby mail and passengers were not properly distinguished in terms of which class that they would be assigned on generation. The old mail with an excessive class will linger until it is delivered, but no new mail with the wrong class will now be generated.
To answer your questions above, the issue with the capacity of classes being shown in the sort by class detail/via displays as the total class seems to be caused by the use of current.menge as the third argument in add_ware_heading() on line 450 of freight_list_sorter.cc, and that simply gives the whole capacity irrespective of class. This is an illustration of why it is difficult for two people to work on the exact same piece of code: it is not clear what was intended here. How did you intend to calculate and store the figure for the per class capacities in order to pass to the add_ware_heading() method? If you want the capacity just for the class to be displayed here, that is what you will need to do.
Adding the parameter g_class to freight_list_sorter (which is currently unused) was intended to allow it to operate exclusively on one class and ignore others, but it became apparent that this was not an effective means of dealing with at least the immediate issues. As the intention behind the code was not entirely clear, I left it in in case this becomes useful. If you find that this has no use in any permutation, feel free to remove it.
To answer your final question, it might be better for you to look into the code, as I really have a very large amount of work to try to get the pakset ready for these new features.
Thank you very much indeed for your ongoing work on this - it is much appreciated.
Hehe, it seems that you ran into the very same issue as I did then with the amount of passengers in each class.
But thanks for the explanations! I will continue working to find some solutions! :)
So, finally!
First of all, it seems as some terminology had been mixed up. Your implementation of g_class in the freight_list_sorter appears to try to sort by the classes of the good them selfs, that would say the wealth of the passengers/mails. That, however, is already possible since the ware.get_class() is accesable and sortable.
The difficult issue where however to be able to sort by class, or accommodation to help distinguish it from wealth.
I have therefore renamed it in the freight_list_sorter to accommodation, and I have had to make some modifications so as to not mix ware.g_class() with accommodation, as they have nothing really to do with each other in this sense.
So now it should (hopefully) work like this:
For both convoy info and halt info:
* Sort by wealth detail/via will sort the passengers and mails to their own wealth, irrespectively of accommodations or anything else.
* Sort by any non-class/wealth sort modes will clump passengers and mails together like they used to do
In convoy info:
* Sort by wealth will first show a line with the total amount of passengers/mails onboard, and underneith the pass/mail sorted by wealth.
* Sort by accommodation will sort the passengers and mails in their specific accommodations they are traveling in.
How it is done:
I have simply moved the sorting of accommodations outside the freight_list_sorter into simconvoi.cc, since we have no way of telling in which accommodation a good is located when it first has arrived to freight_list_sorter. Two new sort options has been created, and simconvoi sorts to different lists of goods dependent on which sort option is created. Well inside the freight_list_sorter, the good are sorted similar as to by_destination_detail and by_via.
To accomplish a correct maximum amount of pass/mail per class, I have created further to the sort_freight:
void freight_list_sorter_t::sort_freight(vector_tpl<ware_t> const& warray, cbuffer_t& buf, sort_mode_t sort_mode, const slist_tpl<ware_t>* full_list, const char* what_doing, const uint8 accommodation, const uint32 accommodation_capacity )
The accommodation_capacity is used to take a single capacity. The function is called once for every class in a convoy when it sorts by accommodation, and one last time for the rest of the good.
Known issues:
* When the vehicle-/halt info window is opened, the sorting is all mixed up. To resolve, just sort to something else and return
* On some sort options, when the convoy is empty, the formatting of the screen looks different
Code can be found here (new branch): https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/new-gui-classes (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/new-gui-classes)
Please try it out and let me know if you find any issues! :)
This works a lot better - thank you for your work on this. I have incorporated this into my passeger-and-mail-classes branch, and also pushed some improvements to make it more likely that passengers of a higher class will choose to travel in higher class accommodation (this is now randomised and the range of random numbers can be set in simuconf.tab).
One thing that I do notice, however, is that the "sort passengers and frieght by" text is missing from the stop information window (although it is present on the convoy information window). Also, there is some inconsistency in the capitalisation of "Wealth" compared with "destination", "via", etc..
This does seem to work well apart from that, however.
I dont think that there ever has been a text "sort passengers and freight by"? Im playing a game from the master branch and there is no such label in the halt info. Instead it says "Goods and passengers waiting:"
The capitalisations of wealth, is due to there being no translations at all for the other sort options, or at least not for all of them. I can change the ones I created to lower case.
Im happy that you find it working! :)
What are left for me to do now with the passenger and mail classes feature?
The reason that I asked about the sort passengers and freight by text is that the dropdown menu does not make much sense without that as a context (players have no way of knowing what "destination", "via", "class", etc. mean without context). I cannot remember whether there used to be context - perhaps this issue was less obvious when it was a button rather than a dropdown menu. Would you be able to add that text so that it makes sense?
As to the things left to do, there are the known issues identified above, the text and capitalisation issues discussed in the immediately preceding paragraph, and a finalisation (if anything is yet to be done) of the .dat file for the translation texts so that I can upload this. One small thing is that I should like to rename "class manager" to "change prices" (at least, the button for it) to make it clearer to players what the feature does (it is not immediately obvious how one would manage a class or why one would want to do so).
In terms of the passenger and mail classes branch itself, I still have quite a lot of pakset work to do, although I am making reasonable progress with this.
Thank you very much for your work on the GUI for this - it has enabled this feature to be much closer to completion much sooner than it would otherwise have been, and probably with a better designed GUI, too.
So, the first fix is online on my branch and it deals with not showing the capacity when the convoys are empty.
I think this works in all circumstances, but maybe you could give it a go too to check if it works consistently!
I have not had a chance to test fully, but this does seem an improvement so far - thank you for your work on this.
I made some small fixes which adresses some of the issues as well as some other issues I found.
One issue though that might need to be adressed:
If you sort by class and change a class, all pass or mail in those accommodations dissappear from the window.
I dont really know how to tackle that, the best thing would be if this was not a GUI-principle but rather a game-design principle that a class could not be changed until it reach a stop, so everybody can "buy new tickets" based on the new accommodations or something similar.
Fixes are online here: https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/new-gui-classes (https://github.com/VictorErik/Simutrans-Experimental-Ves/tree/new-gui-classes)
I have made a small fix on my branch which renames the sort-label to show "Sort passengers/freight by" similarly as to in the vehicles.
Now the backside of waiting with all translations shows its face, because I *think* I have got them all already, but I am not sure. If anyone finds any untranslated text, which btw should be easy to spot, since they have underscores "_" instead of normal spaces, please come forward. Single words have no underscore but might look funny because of no capitalisation and therefore untranslated.
But I do think that you can wrap the datfile up now and upload it to simutranslator.
Thank you for that - now incorporated. However, I do not see any amendments to the .dat file for translations in your most recent commit - can you confirm that you have committed this?
I have not committed any new texts, as I did not find any text that wasnt already translated. I did a big translation pass a while ago updating the dat-file and the translation file at the same time.
Splendid, thank you for clarifying.
Hmm.. there appears to be some strange issues with the class manager. I tried to optimize it a bit by not using fixed amount of pointers, but rather a dynamic one. Or if I messed up the explanations, I have replaced stuff like this:
uint32 pass_capacity_at_accommodation[ 255 ] = { 0 };
with this:
uint32 *pass_capacity_at_accommodation = 0;
Also I have putted these kinds of stuff in the header file so I can reuse the values and dont have to compute them every time.
Problems that arise:
* Window flashes when opening -> Must have something to do with I am calculating the window size and stuf inside it on a fly when the window opens. Minor issue, if considered issue at all.
* Randomly when opened, some phantom classes have sneaked inside with some astronomous values -> Must have something to do with what I wrote in the beginning of post.
* When opening multiple times, the game crashes.... :redx: -> Turfit talked earlier about deleting stuf after one have used it, but I dont know. When trying in debug mode, it doesnt crash!
Do you have any suggestions or advices to the two lower points?
I have uploaded the newest vesrion to the github branch
I cannot reproduce this with a debug version. It would take a considerable amount of time to look into this sort of error. May I suggest simply reverting the optimisation? Since this is GUI code, it is very unlikely that the old code would ever make any noticeable difference to performance (unless you noticed a difference?), as it would only be called once each time that a player opened the relevant window, rather than many times over constantly. Because the old code used all locally allocated variables, there would be no issue with memory leaks, although this code looks as though it allocates new memory every time that the class manager window is opened, but never uses the memory allocated by a previous opening of the class manager window, nor deletes that memory, creating a leak.
Incidentally, running Dr. Memory on the latest debug executable produces the following which might help you to diagnose the problem:
Dr. Memory version 1.11.0 build 2 built on Aug 29 2016 02:42:07
Dr. Memory results for pid 11672: "Simutrans-Extended-debug.exe"
Application cmdline: "C:\Users\James\Documents\Development\Simutrans\simutrans-extended-sources\simutrans\Simutrans-Extended-debug.exe"
Recorded 115 suppression(s) from default C:\Program Files (x86)\Dr. Memory\bin\suppress-default.txt
Error #1: UNADDRESSABLE ACCESS beyond heap bounds: writing 0x3bedbf5c-0x3bedbf60 4 byte(s)
# 0 vehicle_class_manager_t::layout [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:218]
# 1 scr_size::operator scr_coord [c:\users\james\documents\development\simutrans\simutrans-extended-sources\display\scr_coord.h:114]
# 2 gui_scrollpane_t::set_size [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_scrollpane.cc:87]
# 3 gui_frame_t::resize [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:173]
# 4 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:149]
# 5 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
# 6 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
# 7 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
# 8 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
# 9 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
#10 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
#11 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
#12 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
#13 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
#14 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
#15 sysmain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys.cc:825]
#16 WinMain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys_w.cc:1022]
Note: @0:00:51.496 in thread 18224
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x3bedbf58-0x3bedbf5c
Note: allocated here:
Note: # 0 replace_operator_new_nothrow [d:\drmemory_package\common\alloc_replace.c:2913]
Note: # 1 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:62]
Note: # 2 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
Note: # 3 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
Note: # 4 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
Note: # 5 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
Note: # 6 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
Note: # 7 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
Note: # 8 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
Note: # 9 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
Note: #10 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
Note: #11 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
Note: instruction: mov $0x00000000 -> (%ecx,%edx,4)
Error #2: UNADDRESSABLE ACCESS beyond heap bounds: writing 0x3befecd4-0x3befecd8 4 byte(s)
# 0 vehicle_class_manager_t::layout [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:222]
# 1 scr_size::operator scr_coord [c:\users\james\documents\development\simutrans\simutrans-extended-sources\display\scr_coord.h:114]
# 2 gui_scrollpane_t::set_size [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_scrollpane.cc:87]
# 3 gui_frame_t::resize [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:173]
# 4 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:149]
# 5 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
# 6 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
# 7 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
# 8 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
# 9 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
#10 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
#11 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
#12 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
#13 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
#14 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
#15 sysmain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys.cc:825]
#16 WinMain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys_w.cc:1022]
Note: @0:00:51.502 in thread 18224
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x3befecd0-0x3befecd4
Note: allocated here:
Note: # 0 replace_operator_new_nothrow [d:\drmemory_package\common\alloc_replace.c:2913]
Note: # 1 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:71]
Note: # 2 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
Note: # 3 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
Note: # 4 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
Note: # 5 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
Note: # 6 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
Note: # 7 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
Note: # 8 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
Note: # 9 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
Note: #10 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
Note: #11 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
Note: instruction: mov $0x00000000 -> (%eax,%ecx,4)
Error #3: UNADDRESSABLE ACCESS beyond heap bounds: writing 0x3beed614-0x3beed618 4 byte(s)
# 0 vehicle_class_manager_t::draw [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:424]
# 1 win_draw_window_title [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:344]
# 2 display_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:924]
# 3 display_all_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:957]
# 4 win_display_flush [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1547]
# 5 intr_refresh_display [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simintr.cc:79]
# 6 karte_t::sync_step [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:4658]
# 7 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10270]
# 8 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
# 9 sysmain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys.cc:825]
#10 WinMain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys_w.cc:1022]
Note: @0:00:51.574 in thread 18224
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x3beed610-0x3beed614
Note: allocated here:
Note: # 0 replace_operator_new_nothrow [d:\drmemory_package\common\alloc_replace.c:2913]
Note: # 1 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:61]
Note: # 2 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
Note: # 3 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
Note: # 4 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
Note: # 5 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
Note: # 6 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
Note: # 7 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
Note: # 8 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
Note: # 9 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
Note: #10 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
Note: #11 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
Note: instruction: mov $0x00000000 -> (%eax,%ecx,4)
Error #4: UNADDRESSABLE ACCESS beyond heap bounds: writing 0x3bef8fec-0x3bef8ff0 4 byte(s)
# 0 vehicle_class_manager_t::draw [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:428]
# 1 win_draw_window_title [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:344]
# 2 display_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:924]
# 3 display_all_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:957]
# 4 win_display_flush [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1547]
# 5 intr_refresh_display [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simintr.cc:79]
# 6 karte_t::sync_step [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:4658]
# 7 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10270]
# 8 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
# 9 sysmain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys.cc:825]
#10 WinMain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys_w.cc:1022]
Note: @0:00:51.579 in thread 18224
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x3bef8fe8-0x3bef8fec
Note: allocated here:
Note: # 0 replace_operator_new_nothrow [d:\drmemory_package\common\alloc_replace.c:2913]
Note: # 1 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:70]
Note: # 2 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
Note: # 3 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
Note: # 4 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
Note: # 5 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
Note: # 6 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
Note: # 7 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
Note: # 8 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
Note: # 9 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
Note: #10 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
Note: #11 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
Note: instruction: mov $0x00000000 -> (%edx,%eax,4)
Error #5: UNADDRESSABLE ACCESS beyond heap bounds: writing 0x2269c824-0x2269c828 4 byte(s)
# 0 vehicle_class_manager_t::draw [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:424]
# 1 win_draw_window_title [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:344]
# 2 display_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:924]
# 3 display_all_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:957]
# 4 win_display_flush [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1547]
# 5 intr_refresh_display [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simintr.cc:79]
# 6 karte_t::sync_step [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:4658]
# 7 interrupt_check [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simintr.cc:111]
# 8 karte_t::step [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:5392]
# 9 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10368]
#10 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
#11 sysmain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys.cc:825]
#12 WinMain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys_w.cc:1022]
Note: @0:01:21.361 in thread 18224
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x2269c820-0x2269c824
Note: allocated here:
Note: # 0 replace_operator_new_nothrow [d:\drmemory_package\common\alloc_replace.c:2913]
Note: # 1 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:61]
Note: # 2 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
Note: # 3 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
Note: # 4 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
Note: # 5 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
Note: # 6 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
Note: # 7 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
Note: # 8 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
Note: # 9 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
Note: #10 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
Note: #11 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
Note: instruction: mov $0x00000000 -> (%eax,%ecx,4)
Error #6: UNADDRESSABLE ACCESS beyond heap bounds: writing 0x0eea7fdc-0x0eea7fe0 4 byte(s)
# 0 vehicle_class_manager_t::draw [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:428]
# 1 win_draw_window_title [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:344]
# 2 display_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:924]
# 3 display_all_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:957]
# 4 win_display_flush [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1547]
# 5 intr_refresh_display [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simintr.cc:79]
# 6 karte_t::sync_step [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:4658]
# 7 interrupt_check [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simintr.cc:111]
# 8 karte_t::step [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:5392]
# 9 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10368]
#10 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
#11 sysmain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys.cc:825]
#12 WinMain [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simsys_w.cc:1022]
Note: @0:01:21.367 in thread 18224
Note: refers to 0 byte(s) beyond last valid byte in prior malloc
Note: prev lower malloc: 0x0eea7fd8-0x0eea7fdc
Note: allocated here:
Note: # 0 replace_operator_new_nothrow [d:\drmemory_package\common\alloc_replace.c:2913]
Note: # 1 vehicle_class_manager_t::vehicle_class_manager_t [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\vehicle_class_manager.cc:70]
Note: # 2 convoi_detail_t::action_triggered [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\convoi_detail_t.cc:317]
Note: # 3 gui_action_creator_t::call_listeners [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_action_creator.h:36]
Note: # 4 button_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_button.cc:234]
Note: # 5 gui_container_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\components\gui_container.cc:198]
Note: # 6 gui_frame_t::infowin_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\gui_frame.cc:149]
Note: # 7 check_pos_win [c:\users\james\documents\development\simutrans\simutrans-extended-sources\gui\simwin.cc:1425]
Note: # 8 interaction_t::process_event [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:366]
Note: # 9 interaction_t::check_events [c:\users\james\documents\development\simutrans\simutrans-extended-sources\siminteraction.cc:439]
Note: #10 karte_t::interactive [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simworld.cc:10231]
Note: #11 simu_main [c:\users\james\documents\development\simutrans\simutrans-extended-sources\simmain.cc:1362]
Note: instruction: mov $0x00000000 -> (%edx,%eax,4)
I have truncated this, as posting it in full exceeds the post's length limit. These unaddressable accesses are probably what are causing crashes in the release versions.
Incidentally, I have uploaded the translation texts to Simutranslator.
Thanks for the output. I see there is something at line 70(?) in the class manager.
Ok, so then I will revert it back to use the static memory. I'm not home now, but tonight I should be able to.
Out of curiosity, how do you delete alternatively reuse the stuff in the window?
If you want to delete a piece of memory allocated to the heap, just use the "delete" keyword (making absolutely sure that you do not try to access the memory again after it has been deleted, or else the game will crash). If you want to re-use the memory, you will have to (1) store the pointers to the memory somewhere between uses of the window; and (2) know before trying to use the stored pointers whether or not the memory to which they point has been initialised. In this sort of case, it would be easier just to delete them, as this is not a performance critical part of the code.
For more information on using the new/delete operators in C++, see this (http://www.cplusplus.com/doc/tutorial/dynamic/) tutorial.
Thanks, I understood that I had to use the delete command. What I do find difficult however is to place it correctly in the code.
What I want it for is for the comboboxes. Turfit mentioned that it would probably be wise to not have 255*2 comboboxes prepared, if only a few had a chance of being used.
In what function should I place the delete commands you think? What part of the code handles when the window is closed?
Quote from: Ves on November 26, 2017, 07:58:42 PM
Thanks, I understood that I had to use the delete command. What I do find difficult however is to place it correctly in the code.
What I want it for is for the comboboxes. Turfit mentioned that it would probably be wise to not have 255*2 comboboxes prepared, if only a few had a chance of being used.
In what function should I place the delete commands you think? What part of the code handles when the window is closed?
The trouble is that to answer that question, I should have to review that part of the code in very great detail, and quite possibly undertake extensive testing.
Did you notice any problems with the old code? Did TurfIt explain why having 255 comboboxes in memory while this window is open would cause problems?
Well, he just said that it would be better to allocate and remove instead of booking so much memory. However, I think I found out how to delete them again creating this:
vehicle_class_manager_t::~vehicle_class_manager_t()
{
for (int i = 0; i < pass_class_sel.get_count(); i++)
{
if (pass_class_sel.at(i))
{
delete pass_class_sel.at(i);
}
}
for (int i = 0; i < mail_class_sel.get_count(); i++)
{
if (mail_class_sel.at(i))
{
delete mail_class_sel.at(i);
}
}
}
and then in a for loop delete all comboboxes which was created.
I think it works, and my experiments with breakpoints suggests that this piece of code is accesed whenever the window is closed.
New version on the github branch together with some other small fixes!
Splendid, thank you for that; now incorporated. May I ask whether base-texts-classes_gui_2.dat is ready to upload yet?
Well, I cant think of any more translations just yet. :P
I whish however that there was a simpler way of doing it with those files, so one dont have to create new files for just some words, clogging up the folder.
Is there not any other way it could be done, so one could just only be updating one single file?
I am not aware of any other way of doing it: the files need to be compatible with the Simutranslator interface, and that is run by Frank, rather than by me. I will upload the new file now. Incidentally, have you done any Swedish translation on these new texts?
I have not done any swedish translations of several reasons, the biggest reason being that it is a gigantic work to begin with. Secondly, as the game developes, there will allways come new texts to translate so I would never feel finish with it, and lastly I have not gotten around to setup an account on simutranslator. Also, the paksweden has alot of translation to be done, both english and swedish and I think I would rather have it all in one go.
When I play, I play in the english language.
That seems reasonable - I was asking in case I needed to import any texts. I imagine that there are higher priority tasks.
Thanks for asking, and you are right, other tasks, like painting the actual stuf, has a higher priority :P
I approve of actual stuff.