Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add: the ability to use multiple train plugins #541

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

s520
Copy link
Contributor

@s520 s520 commented Oct 3, 2020

This PR is an extension of the PR submitted to my repository of plugin that runs multiple train plugins.
s520/DetailManager_For_OpenBVE#1

This PR allows:

  • Multiple legacy and .NET plugins can be used in any combination at the same time.
  • Panel and sound indexes and key settings can be mapped. (only "ats.json")
  • When editing the configuration file, the input completion and verification of the editor (like VSCode) by JSON schema available.

Here are some things to consider:

  • File format.
    • Are there any other items that are necessary or unnecessary?
  • Testing is still lacking.
    • Especially related to sound.

e.g. ats.json (Comments can not be used.)

{
    "type": "Ats",
    "version": "1.0",
    "plugins": [
        {
            "filePath": "RelativePathTo/plugin1.dll",
            "panelIndexMap": {  // Indexes that are not described are mapped in the same way as on the plugin side.
                "offset": 10,  // Adds the number specified in offset to the index of the plugin. It does not apply to the plugin side index described in the entries.
                "entries": [
                    {
                        "src": 5,    // Index on the plugin side
                        "dest": 45   // Index on the main sim siede
                    }
                ]
            },
            "soundIndexMap": {},
            "keyMap": {
                "entries": [
                    {
                        "src": "L",
                        "dest": "J"
                    },
                    {
                        "src": "None",  // The "None" in the key map and the "-1" in the index map mean that they are not mapped.
                        "dest": "L"
                    }
                ]
            }
        },
        {
            "filePath": "RelativePathTo/plugin2.dll"
        }
    ]
}

cc @zbx1425

@leezer3
Copy link
Owner

leezer3 commented Oct 3, 2020

First, a thought:
Are you approaching this in slightly the wrong way?

Plugins tend to do two things, i.e:

  1. Simulate safety system(s)
  2. Simulate traction, windscreen etc.

I'm basically wondering whether a two-plugin approach would be better, one to simulate safety systems, and one to simulate traction modelling as reqiured.
This would then allow most things to be kept unchanged, with appropriate accessors added for the traction plugin, and some sort of way to mediate control inputs between the two.

Secondary thoughts:
A lot of what plugins do should be implemented into the main sim; For example, the windscreen / raindrops shouldn't need a plugin to handle them.
I'll try and think about some windscreen code this week (it's already in BVEC_ATS, just needs cleaning up probably), and it may be worthwhile considering exactly how much we actually need a plugin for, and what needs implementing in the main sim.

@s520
Copy link
Contributor Author

s520 commented Oct 3, 2020

In BVE5, it is common to have multiple safety devices as separate plugins.
Train authors can easily simulate trains with multiple safety devices by combining them.
Most importantly, this can be achieved without the train author having any programming skills.
Since we have programming knowledge, it will be easy to combine multiple safety devices into one plugin.
But for many train authors it is difficult.
We should provide a means for those people.

Also, as you say, I think it is better to add functions that can be shared by many trains such as wipers to the main program.

@leezer3
Copy link
Owner

leezer3 commented Oct 3, 2020

That's interesting & a reasonable answer.
Not sure I personally agree with the exact approach, but if BVE5 allows it then it will probably be OK.

In principle, I'd like to bring most code which requires a plugin into the main sim, but that's not going to be achiveable in the short (or probably longer term), and achieving exact prototypical behaviour is always going to to be a problem.

@leezer3
Copy link
Owner

leezer3 commented Oct 3, 2020

#541

That's the basic code for windscreen wipers.
Still needs the configuration and a few other bits and bobs doing to it.

@zbx1425
Copy link
Contributor

zbx1425 commented Oct 3, 2020

I think @leezer3 might favor an ats.xml file, since most of the new things are in XML now, and Newtonsoft.Json seems to be a heavy library (size after built is a little bit large, performance should be ok though)
Or maybe one would argue the XML syntax is too redundant. In that case providing JSON (or YAML/TOML) support for other elements (train.json, etc.) might be a practical idea.

Supporting multiple train plugins is definitely a must-have from my perspective.
I also suggest extending panel/sound variable count limit. and probably let the ATS plugin integrate better with the panel touch function, such as retrieving click position, the mouse button pressed, and scroolwheel movements. (Probably void panelElementClicked(object sender, MouseEventArgs e))

@leezer3
Copy link
Owner

leezer3 commented Oct 3, 2020

Good point- We need to be consistant on formats.

I really wouldn't reccommend YAML, syntatic whitespace is an absolute utter idiotic way of doing things.

When I first started this 4+ years ago, we needed a consistant external format; BVE's internal formats have gotchas I'm still finding today....
At the time, XML was the most common choice for this sort of thing (and in use by stuff such as LokSim, TrainSimulator etc.), hence why it got picked & TOML was in an early beta of some description.

IMHO, there's nothing wrong with redunancy either, as it clearly shows what's going on.

TLDR:
XML was picked ages ago, probably far too much effort to change again (unless you're volunteering!)

@s520
Copy link
Contributor Author

s520 commented Oct 3, 2020

To be honest, XML is too verbose for humans to write.
My friends say it's a hassle to write the same letters when enclosing them in tags.
When I showed this JSON, he said it was concise and very good.

It's understandable to worry about more formats.
To solve both problems, we would need to be able to do input completion and validation in the editor when editing the XML.
Does XML Schema have the same functionality and convenience as JSON Schema?

@zbx1425
Copy link
Contributor

zbx1425 commented Oct 3, 2020

The major verbosity is caused by the tag name being written twice. JSON is more concise at this.
And also the <?xml version="1.0" encoding="UTF-8"?>.

For me personally, I think JSON is a better choice, but XML is not that bad.
I have a route developer friend who recently tried out TFO for the first time, and he got into lots of problems (open/close tag forgotten, etc.), then complained that the XML format is a lot stricter compared to the BVE formats. Although it got better after learning, it is obvious that it might cause some problem for newcomers.

Also parsing errors in some XML files seem uncaught, and will crash the simulator instead of reporting an error. Is that fixed now?

BTW thanks for inviting me.

@zbx1425
Copy link
Contributor

zbx1425 commented Oct 3, 2020

Also, do you consider it necessary to implement a backward-compatibility plugin?
That is, a DetailManager.dll that can parse the ats.xml (or ats.json) file.

ats.cfg --> DetailManager.dll assigned for backward-compatibility
DetailManager.dll --> Multiple plugin loader - so that legacy openBVE can achieve the same function
ats.xml --> new OpenBVE ignores ats.cfg if ats.xml is present
xxx.dll, yyy.dll --> other dll plugins are referred by ats.xml

@leezer3 leezer3 added this to the 1.8.0 milestone Nov 6, 2020
@leezer3 leezer3 force-pushed the master branch 5 times, most recently from bad6395 to 438f090 Compare November 20, 2020 11:41
@HUN-Droid
Copy link

@zbx1425

Supporting multiple train plugins is definitely a must-have from my perspective.
I also suggest extending panel/sound variable count limit. and probably let the ATS plugin integrate better with the panel touch function, such as retrieving click position, the mouse button pressed, and scroolwheel movements. (Probably void panelElementClicked(object sender, MouseEventArgs e))

In the .NET asseblies you can override the default size Panel (Panel = new int[newSize];), and can create a bigger size array. I use over 1000 size array as Panel, and it works absolutely perfect (also above 255).
I don't have experience with the same solution at sounds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants