leezer3 / OpenBVE

OpenBVE- A free train simulator
http://www.openbve-project.net
281 stars 51 forks source link

[Suggestion] Multiple consists for a single train #50

Open marcriera opened 8 years ago

marcriera commented 8 years ago

I have come up with a possible way of implementing consists using the current train format.

Currently the train format only allows to have a single consist per train. If we wanted to have different liveries of the same train, or double consists, for example, we would have to create a new train for each. Some developers have been using subfolders to group these consists (Chashinai Railway is the best example probably), but it is not the best solution,

The idea is quite simple. Using a new "consists.cfg" file placed in the train folder, we define the consists:

[Consist0] Name = Default

[Consist1] Name = Custom Consist 1 Folder = con1

etc.

Each [Consist] section defines a different consist. 'Name' defines a name for the consist (so it can be shown in a list in a new tab in the train selection menu, for example), and 'Folder' sets the consist subfolder relative to that train's folder. Each consist may have custom train files (Train.dat, extensions.cfg, etc), but it is not necessary to have all of them (in case there is no Train.dat in the subfolder, for instance, the default one in the 'root' folder is used). This keeps the new file simple yet powerful.

Backwards-compatibility should not be an issue, as old versions of the program would simply ignore the new file. [Consist0] would not have a 'Folder' setting because it would always use the standard files in the 'root' folder (so old versions openBVE just read it as a normal train). As far as routes are concerned, adding a new parameter to the Train.Folder command would be enough to tell the game if a custom consist is used. Say you want to use [Consist4] of the LT1995 train, for example. It would look like this:

Train.Folder(LT1995;4)

I have tested this in the current build of openBVE and the ';4' part is simply ignored with no warnings or errors, so a future implementation would be 100% painless.

I have some C# skills and I could even code this myself, but I want to know first if you think it is a good idea, or if there is any part of it that could be improved. Thank you!

mgavioli commented 8 years ago

Let me dream and see if we can push this a little forward.

If the consists.cfg, in addition to (or instead of?) ready-made consists, details the individual cars (in a way to decide, extensions.cfg might be a starting point), it would be possible to allow the user to build its consists 'on the fly' (and perhaps, save and re-load them). Lot of UI to build, but this would bring openBVE at the same level as the most 'emblazoned' train simulators...

Being in a new .cfg, this would be completely invisible to old programme versions and would give new versions total freedom about implementation details.

leezer3 commented 8 years ago

I used the subfolders concept in the most recent release of the 81xx, and it definitely is not ideal.

Perhaps what this needs though is a slight paradigm shift: You're thinking in terms of the route-file selecting the consist- What might perhaps be better is a dropdown box after route/ train selection to select the 'consist' Potentially, your routefile solution could also be used to select the default consist that a train provides.

Ideally though, we need to fix the situation with moving bogies, especially if user-built consists are being considered. Currently, in order to make a bogie articulate correctly with curves, it's necessary to define the bogie using it's own [Car] section in the extensions.cfg file, and use one of two tricks to move it into position. Of course, some trains just leave out moving bogies altogether......

From a brief dig into the code, I think it's probably possible to add four more TrackFollowers to each car (Representing the front and rear axles of each bogie) Most of the heavy lifting code is in TrackManager.UpdateTopplingCantAndSpring() and it looks to me as if this could be extended without too much trouble. The problem I suspect is more in the parser and the renderer. I'll need to do some more digging/ stepping through in the debugger.

mgavioli commented 8 years ago

Possibly, in order to understand, I need some more elaboration.

1) I understand that the moving bogie issue has to be addressed sooner or later per se; what I do not grasp is how this issue would become worse than it is now in the case of multiple pre-defined consists or of user-created consists (sooner or later, I will need a crash course on bogies in openBVE, though...).

2) Indeed, what I was thinking about was a further user choice for the consist. It might be a choice among a number of ready-made consists prepared by the developer (as per @MarcRiera proposal) or an interface allowing to compose a new one (as I dream of) or both.

3) It could make sense that the route still defines a default train/consist, for backward compatibility. A default which a new version could easily ignore to unconditionally present the consist selection.

marcriera commented 8 years ago

The bogie issue certainly needs a solution because the current way of developing trains with moving parts is terrible. I have developed many 5-car EMUs with articulated bogies and gangways, and the only way to do this is to create a 19-car train (5 cars + 10 bogies + 4 gangways). This requires train mass recalculation in Train.dat in order to get the same train performance.

User-made consists would be great, but I wonder whether it is feasible. Each consist (if we talk about true consists and not only paint liveries) is basically a new train and it would require custom Train.dat, so maybe it would be simpler to create a separate developing tool to add consists to an existing train (and help creating a extensions.cfg file from a custom set of train objects.)

I agree with @leezer3 when it comes to how consists would be selected. After the route and train have been selected, the player would be asked which consist to use. We could use a new dialog, or simply add the option to the train section of the main form. In addition to that, routes could potentially select a custom consist by default, but the option to override it should always be available (unlike MSTS and activities, that don't allow to change consists).

leezer3 commented 8 years ago

I'd say no new dialogs, unless it's for something like editing consists.

Keep it as simple and close to the original as possible, until and unless it's absolutely required.

Will try and get to this in the debugger this week.

leezer3 commented 8 years ago

OK, some progress on bogie motion! bogies

The branch above loads bogies, and works mostly as designed. It's mostly just a nastily hacked around version of Michelle's car motion code, but if it works, who cares! At present, a standard two-bogies per car implementation is all that's going to be supported, no gangways or anything fancy (Sorry!)

One fundamental problem remaining at the minute- Z-sorting/ render order seems to get stuffed up sometimes. I normally notice this on the player's train, and I suspect it's probably related to having two trains in view at the same time. Please excuse the cubes, it's easiest to see bugs with objects like this! renderbug

I haven't got to the bottom of this one just at the minute unfortunately. I suspect that something somewhere is sorting based upon the driver's car and a set of assumptions, but will need to go digging into the renderer again.

Optimisations or suggestions anyone?

mgavioli commented 8 years ago

For me, understanding the code (the original one and the changes) is going to take a while...! I see that bogies are still considered separate cars; two questions:

(Incidentally, I notice your commit reverts the last commit merged into master about in-game maps; is this intentional?)

leezer3 commented 8 years ago

Nope, highly unintentional reverting that commit..... Too many local branches floating around.

Some lengthy musings on what this does follow: With this commit, bogies are not considered cars as such, but are rather a subset of the existing car functionality.

At present, you define the train length via the train.dat file- n motor cars and x trailer cars. Each car has a defined weight, which is used for physics calculations etc. These cars move along the track, and are rotated around two followers at the front and back axles. In order to define a bogie, it must have it's own car, and you then need to use tricks to get the physics engine to move it into your desired position.

Assume that we have a four car train. The mass of a car (For simplicity both that of the trailer cars and the motor cars) is set at 25 tons. In order to add bogies to our train, we require eight extra cars. Thus, with the current solution, at the default mass of a car, we've suddenly got 200 tons more in the physics engine than we accounted for.

This commit changes things a bit- The train now has an array of bogies attached, which is exactly double it's length. (I.E. 2 for every car) A bogie is then positioned on the front and rear axle of each car. These bogies have their own TrackFollowers, which are updated to get the world positions of each axle, and thus the car rotated. They have no mass, and essentially follow the root car's toppling/ cant movements, but affected by their own axis of rotation.

Creating problems is a 'relative' term, but you're absolutely right. The obvious place for confusion is the means by which bogies are attached to an existing axle- Bogies are currently independant in the extensions.cfg parser, rather than being attached to a [CarN] section.

[Bogie0]
...Attached to Car0

[Bogie2]
...Atached to Car1

This isn't ideal, as it requires careful counting. However, this shouldn't (Famous last words!) be affecting the Z-sorting order. Shortened version of what should be happening: Train enters camera viewing radius. A series of loops is then run, adding the train's elements to the renderer's list of visible objects. Finally, the renderer itself should be doing the Z-sorting, from here on: https://github.com/leezer3/OpenBVE/blob/master/openBVE/OpenBve/OldCode/Renderer.cs#L854 Most likely explanation is that I've still got something linked to the wrong follower. As noted above, I suspect it's going to be something that links off the driver car's index, but the trouble is that there's loops calling loops, calling loops.

leezer3 commented 8 years ago

Some more thoughts.....

Editing consists in and of itself is relatively easy with this solution- All that would be required would be a simple set of index replacements within the extensions.cfg file (Or similar replacement)

What isn't quite so easy, is allowing the attachment or detachment of car(s) on the fly, which I presume is the ultimate aim, and what you're referring to.

Stopping the motion of the physical objects themselves I think should be relatively easy (Again, famous last words!), as all we need to do there is to stop updating the TrackFollowers for the appropriate cars. Similarly, the acceleration curve can be altered by fiddling with the mass numbers appropriately- Whilst I haven't dug into this in a major way, I don't see any real problem with it.

Your problems come about when you detach one car, and insert another car in it's place- Before long with the current situation, you'll end up with a train array containing cars all over the place.

What it really requires, is the alteration of the Train concept a little, to something which I suspect is rather more conventional- Rather than the simulation consisting of X number of trains, made up of Y number of cars, the model should instead be inverted, so that the simulation consists of cars placed within the 3D world, which are updated as appropriate by whichever train they are attached to. Thus, a train would look something more like this:

Train 1:
Car 3
Car 18
Car 27

Train 2:
Car 4
Car 9
Car 15

etc. etc.
marcriera commented 8 years ago

I am amazed with the progress, good job! Gangways are not a priority (if we even consider them as a problem), but the bogies really were something badly implemented and you managed to make them better. I will make sure I update my content to use this new feature.

The idea of placing individual cars and then "grouping" them to allow consists sounds good, but I just can't imagine coupling and uncoupling cars with the existing route format. I'm not saying that it's impossible, or that we need to redesign formats right now, but that maybe it looks too far in the future for now.

Replacing indexes through extensions.cfg seems way more feasible, though.

leezer3 commented 8 years ago

I warn you, this may well have unexpected bugs!

I haven't even tried testing animations yet, which may well throw a spanner into the works, as I haven't dug into how exactly they get the car index based speed etc. data. (In theory, this shouldn't matter, but in practice anything can happen.....)

marcriera commented 8 years ago

Don't worry, I am aware of the possible bugs, but trying it is the best way to find them. Will test this in the next few hours to see if something is broken.

Also, for some reason, it seems like I'm some sort of magnet when it comes to bugs, I always find something!

mgavioli commented 8 years ago

With regrets, I doubt I will have time for any meaningful test until Saturday, so I'll leave this to @MarcRiera (who is also definitely more knowledgeable than I am).

Detaching and adding cars on the fly during the simulation (i.e. shunting, I presume) might be the ultimate goal, but very ultimately, I believe!

I think that either or both the features described initially, in summary:

would already be huge bonuses, even if the consist, once chosen or made, cannot be modified during the simulation.

The reversal of the trains-cars relationship you describe might make the implementation of either feature (particularly the second, I guess) cleaner and more intuitive; but, conceptually, seems to me little (little?) more than replacing a matrix with its transposed (and modifying accessors accordingly).

leezer3 commented 8 years ago

I agree that in it's current state, changing from a Train based model to a car based model is not much more than transposing matrix states. Detaching cars from the end of the train is probably actually relatively simple, but attaching cars is not going to be.

Technical thoughts on car vs. train design follow: Consider a simple multiple train situation using train based references: We start off with two engines plus two cars in sidings. One engine moves, connects to a car, moves it to a new location and finally disconnects.

Already, the current model starts to have troubles: A car is not placed, except as part of a train. How do we determine which train our two independantly placed cars belong to?

Making a basic assumption, and linking them to the nearest train gets us out of the first hole, but only digs us deeper for the second hole.

For moving the cars, the train motion is just fine, and can be handled by some relatively simple code.

We then disconnect, and promptly run into the much bigger hole that I mentioned above. Leaving the car in place (Or for that matter reconnecting) is relatively trivial, but we now want to attach both cars to our train. Thus, our initial assumption falls flat on it's face- Linking the car to the closest train was probably a bad idea...... Re-thinking, and linking all loose cars to the player train solves that little problem, but promptly triggers another problem- If we instead couple to car 2 first, then go back to car 1, what order should they be held in the array and updated?

That's only off the top of my head....

Similarly, the possibility of moving trains on other(opposing) tracks should be thought of in design choices made now, and IMHO a car based model is the only sensible way to do this.

marcriera commented 8 years ago

I have tested this but still haven't managed to get it to work properly. I have noticed that .csv objects get rendered in a similar fashion to cabs and always appear over other objects (even if they are in the correct location). Animated files seem to render correctly, though. This is what I mean:

captura de pantalla de 2016-03-17 23-16-02

I also haven't been able to add more than one bogie in the whole train. Adding [Bogie1] leads to an "Index out of the bounds of the array" error message after the train is loaded (and the program closes).

Details from the debugger:

System.IndexOutOfRangeException: Index was outside the bounds of the array. at OpenBve.TrainManager.UpdateFrontBogieSectionElement (OpenBve.Train Train, Int32 CarIndex, Int32 SectionIndex, Int32 ElementIndex, Vector3 Position, Vector3 Direction, Vector3 Up, Vector3 Side, Boolean Show, Double TimeElapsed, Boolean ForceUpdate) [0x001f2] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/TrainManager.cs:1759 at OpenBve.TrainManager.UpdateFrontBogieObjects (OpenBve.Train Train, Int32 CarIndex, Double TimeElapsed, Boolean ForceUpdate) [0x00518] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/TrainManager.cs:1491 at OpenBve.TrainManager.ChangeFrontBogieSection (OpenBve.Train Train, Int32 CarIndex, Int32 SectionIndex) [0x00169] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/TrainManager.cs:1645 at OpenBve.TrainManager.UpdateTrain (OpenBve.Train Train, Double TimeElapsed) [0x00195] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/TrainManager.cs:1849 at OpenBve.TrainManager.InitializeTrain (OpenBve.Train Train) [0x00026] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/TrainManager.cs:1239 at OpenBve.OpenBVEGame.SetupSimulation () [0x00466] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/NewCode/GameWindow.cs:392 at OpenBve.OpenBVEGame.LoadingScreenLoop () [0x0017f] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/NewCode/GameWindow.cs:629 at OpenBve.OpenBVEGame.OnLoad (System.EventArgs e) [0x0005a] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/NewCode/GameWindow.cs:256 at OpenTK.GameWindow.OnLoadInternal (System.EventArgs e) [0x00000] in :0 at OpenTK.GameWindow.Run (Double updates_per_second, Double frames_per_second) [0x0008f] in :0 at OpenTK.GameWindow.Run () [0x00000] in :0 at OpenBve.MainLoop.StartLoopEx (MainDialogResult result) [0x00386] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/OldCode/MainLoop.cs:125 at OpenBve.Program.Main (System.String[] args) [0x005f4] in /home/marc/Escriptori/OpenBVE-BogieMotion/openBVE/OpenBve/System/Program.cs:240

leezer3 commented 8 years ago

I've just fixed a rather glaring logic failure, which I think is probably at the root of your issue. https://github.com/leezer3/OpenBVE/commit/7dd2b2d8569be30f2dd4cc440bba70044380cdc2

TLDR: In order to get the bogie object array index number, I multiplied the car index number by 2 for the second bogie..... Of course, this appears to work just fine on a 2-car train, but anything longer throws a hissy fit. Should have just used a new incrementing counter in the first place after loading each bogie, no idea why I didn't......

marcriera commented 8 years ago

The new build fixes the crash, but the two bogies of the first car are now displayed as overlays (even if they are animated objects). The others look perfect (tested a train with 5 cars).

captura de pantalla de 2016-03-18 16-58-11

Also, some false warnings are displayed due to a minor coding error. To fix it, just replace the 'if' with an 'else if' on line 206 of openBVE/OpenBve/OldParsers/ExtensionsCfgParser.cs (not worth a pull request).

leezer3 commented 8 years ago

That makes a lot of sense, although I suspect it's the drivers car rather than Car 0 :) I'll bet something is making the assumption that Index 0 of an object array attached to the drivers car is an auto-generated cab object.

Will dig into this tonight or tomorrow AM.

leezer3 commented 8 years ago

Found :+1: The overlay properties for the bogies were being initialised using the properties of the root car. Object #0 of the driver's car is the cab, and hence an overlay.

Have changed it so that bogies are never rendered as an overlay. I've also just fixed the fact that bogies were not hidden when returning to the cab view.

https://github.com/leezer3/OpenBVE/commit/68d28142503863bb9e5bf9eff12006bdca6b58f4

leezer3 commented 8 years ago

Some further musings: This now seems to work reasonably well. (Again, famous last words.....)

It could be extended very easily for gangways/ physical coupler objects. I'd also like to add per-car weights, rather than the averaging that's currently done.

That leads me on to the 'problem'- Is it worth keeping the extensions.cfg structure, or perhaps should we just create an entirely new file format to support this more nicely?

I'm thinking along the lines of a per-car XML file, looking like this:

<?xml version="1.0" encoding="utf-8"?>
<openBVE>
<Car>
<GUID>123456789</GUID>
<Description>XML Test Car</Description>
<Body>
<Object>Object.b3d</Object>
<Length>25</Length>
<Axles>-10,10</Axles>
<Weight>25</Weight>
</Body>
<FrontBogie>
<Axles>-2,2</Axles>
</FrontBogie>
<RearBogie>
<Axles>-2,2</Axles>
</RearBogie>
</Car>
</openBVE>

Consists would be a similar XML file, something like this:

<?xml version="1.0" encoding="utf-8"?>
<openBVE>
<Consist>
<Vehicle>123456789</Vehicle>
<Vehicle>123456789</Vehicle>
<Vehicle>123456789</Vehicle>
</Consist>
</openBVE>

This has the advantage of not interfering with existing files, and provides the option of a legacy fallback mode to extensions.cfg

Thoughts?

marcriera commented 8 years ago

The bogies still appear in the cab view right after the train is loaded, but as soon as you press F1 again or change to another view they disappear. The overlay issue is completely gone now, however.

Probably the best way to keep adding new features to trains is a new format, as you say. Expanding the extensions.cfg is not bad, but I think it's a rather limited to what we want to achieve now and a brand new file would be better. XML files aren't as simple as the current formats, but they are used everywhere, are easier to parse and allow for a tree structure. I'd use them, they are extremely flexible (I have even seen images embedded into XML files).

These new car and consist XML files look good, but maybe we could just use two files: one for cars and the other for consists. Using one XML for each car and consist doesn't really make sense with a tree structure that can be edited easily to add, edit or remove contents.

Properties such as per-car weights are also a good idea, because the current Train.dat-based format is really limited. The same could be done for motor and trailer cars, specially now that consists are being taken into account.

leezer3 commented 8 years ago

Have now found your bogie issue and fixed, although of course I couldn't see it when I merged this into the main tree.....

Haven't started XML train information yet, but am still thinking about it.