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

Fix safety systems error with ai trains #554

Merged
merged 1 commit into from
Mar 9, 2024

Conversation

Jomosoto
Copy link
Member

@Jomosoto Jomosoto commented Mar 4, 2024

On ai trains, the Node of type LTSPlayer is not called "Player" and the safety systems get the player node by name.

This fix disables the Saftey Systems if the player node is null, as they shouldn't be enabled in ai trains anyways. This isn't a clean solution, but it's uncomplicated.

Other solutions would be

  • to export the player variable (you would have to connect it manually, but the PZB signals also have to be connected manually at the moment)
  • or to have it set by the player maybe as part of the SafetySystems class.

The last one is my favourite, as every safety system will have to have access to the player, but I'm not sure and I'd love to hear you opinion

@Jomosoto Jomosoto added the bug Something isn't working label Mar 4, 2024
@Jomosoto Jomosoto added this to the 0.9 milestone Mar 4, 2024
@Jomosoto Jomosoto requested a review from a team March 4, 2024 19:15
@Jomosoto Jomosoto added this to In progress in v0.9 via automation Mar 4, 2024
Copy link
Member

@HaSa1002 HaSa1002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair change. I agree, we should add these systems on request to the train and simply not add them in that case. So yes, option three

@HaSa1002 HaSa1002 merged commit 6db4820 into Libre-TrainSim:master Mar 9, 2024
3 checks passed
v0.9 automation moved this from In progress to Done Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
v0.9
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants