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

Added general compability for tableGenerator (SWTbahn-Game) #132

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

Conversation

noriakisana
Copy link
Collaborator

Changed the tableGenerator to automatically generate all configuration files which are needed for the driver-game, as soon a mappingFile is existing.

@noriakisana noriakisana requested a review from BLuedtke January 24, 2024 12:16
@noriakisana noriakisana self-assigned this Jan 24, 2024
@noriakisana noriakisana requested a review from eyip002 January 24, 2024 12:21
@eyip002
Copy link
Member

eyip002 commented Jan 24, 2024

Have you tested that the Game still works? I don't think the Game works anymore.
How does the Game know which platform it is connected to, so that it uses the correct flag mappings?

The generated destinations-swtbahn-full.json includes the blacklisted routes. You should restore the previous blacklist.txt file and rename it to blacklist-swtbahn-full.txt and update your Python script accordingly.

@noriakisana
Copy link
Collaborator Author

Currently the platforms have to been set manual. For an automatic solution the issue #131 have to be fixed.

@noriakisana
Copy link
Collaborator Author

blacklist feature is now added

@eyip002
Copy link
Member

eyip002 commented Jan 24, 2024

Are you also testing that the initial loading of the game script still works?

@noriakisana
Copy link
Collaborator Author

shouldnt work currently cause the dependency selection is broken (need who am I API).

@eyip002
Copy link
Member

eyip002 commented Jan 24, 2024

For pull requests to be merged, the software needs to run correctly. We can't leave the software in a broken state.
You can create a getPlatformName function in the game script that returns the name of the platform as a string. The return value can be hardcoded while #131 is being worked on.

function getPlatformName() {
  return "swtbahn-full";
}

Use this function in the initialise function to select the correct destinations and mappings to use.

Signed-off-by: Jochen Mehlich <[email protected]>
@noriakisana
Copy link
Collaborator Author

need to be verified next week on hardware

Comment on lines 14 to 15
var allPossibleDestinations = null;
var signalFlagMap = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of removing the comments, improve/adapt them to fit the new situation

Copy link
Collaborator

Choose a reason for hiding this comment

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

probably better to keep the old-style comments, which offer a clearer per-variable explanation (i.e., what is each var for)

server/src/assets/game/script-game.js Outdated Show resolved Hide resolved
server/src/assets/game/script-game.js Outdated Show resolved Hide resolved
server/src/assets/game/tableGenerator/Readme.md Outdated Show resolved Hide resolved
server/src/assets/game/tableGenerator/Readme.md Outdated Show resolved Hide resolved
server/src/assets/game/tableGenerator/Readme.md Outdated Show resolved Hide resolved
server/src/assets/game/tableGenerator/Readme.md Outdated Show resolved Hide resolved
### Map the Signals with the Symbols
* Create a CSV Document with the following Scheme to the flagMappingsFolder (it should have the same name, like the configuration folder): [signalId],[coloCode]
* insert your mapping attributes
* link the csv to generator.py and converter.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand what that means

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not yet resolved, still unclear what this means.

* link the csv to generator.py and converter.py
* run generator.py
* run converter.py
* link the generated files to the game
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does "link" mean here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

still unclear.

@noriakisana noriakisana requested a review from BLuedtke February 14, 2024 09:47
var signalFlagMap = null;

function getPlatformName(){
return "swtbahn-full";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a "/// TODO:" here with a comment to indicate that this will need to be adapted once the corresponding server endpoint exists.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(also will have to think about error handling in the future here, as well as blocking this to only return once ready)

server/src/assets/game/script-game.js Outdated Show resolved Hide resolved
# Table Generator for SWTbahn Game

This python script generates the configuration json files for the swtbahn game client, which are required for the signal-symbol mapping and the route suggestion mechanics.
The script will automatically find a suitable route (which the least nodes) and store it for the game client, it will do this action only for signals which are listed in the mapping-csv to reduce storage usage.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"The script will automatically find a suitable route" -> to where? The latter part of the sentence implies it is done with all signals from the mapping-csv as the source, but it is not quite clear where these routes are leading.

"which the least nodes" -> grammar/incorrect english; rather say something like "find the shortest possible route from ... /(for all..) to ..." or something like that.


## How to get started
### Map the Signals with the Symbols
* Create a CSV Document with the following Scheme to the flagMappings directory (the filename should match with the name of the configuration folder of the model railway): [signalId],[colorCode]
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Scheme" -> "content formatting"

"to the flagMappings directory" -> "in the flagMappings directory"

### Map the Signals with the Symbols
* Create a CSV Document with the following Scheme to the flagMappingsFolder (it should have the same name, like the configuration folder): [signalId],[coloCode]
* insert your mapping attributes
* link the csv to generator.py and converter.py
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not yet resolved, still unclear what this means.

* link the csv to generator.py and converter.py
* run generator.py
* run converter.py
* link the generated files to the game
Copy link
Collaborator

Choose a reason for hiding this comment

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

still unclear.

Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
@noriakisana noriakisana marked this pull request as draft March 12, 2024 08:36
@noriakisana noriakisana removed the request for review from eyip002 March 26, 2024 08:20
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
@noriakisana
Copy link
Collaborator Author

works.

Comment on lines 871 to 876
namedMap = "allPossibleDestinations_" + platform;
str = "allPossibleDestinations =" + namedMap;
eval (str);
namedMap = "signalFlagMap_" + platform;
str = "signalFlagMap =" + namedMap;
eval (str);
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate code?

@@ -796,6 +805,16 @@ function initialise() {
null // trainId
);

var platform = getPlatformName();
Copy link
Member

Choose a reason for hiding this comment

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

const

Comment on lines 810 to 815
namedMap = "allPossibleDestinations_" + platform;
str = "allPossibleDestinations =" + namedMap;
eval (str);
namedMap = "signalFlagMap_" + platform;
str = "signalFlagMap =" + namedMap;
eval (str);
Copy link
Member

Choose a reason for hiding this comment

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

Don't reuse variables (namedMap, str) for different purposes!

Copy link
Member

Choose a reason for hiding this comment

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

Why not limit the evaluation to the rhs of each variable assignment?

allPossibleDestinations = eval("allPossibleDestinations_" + platform);
signalFlagMap = eval("signalFlagMap_" + platform);

Doing:
Roll over all Blocks and search for a RouteID for the destination and insert the details based on start block, destination signal. Data were read from the interlocking table
"""
originalResultData = copy.copy(copy.copy(resultData))
Copy link
Member

Choose a reason for hiding this comment

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

Why double copy?

destinations.append(destination)
destinationsSorted = []
print(destinations)
with open(groupingFile, "r") as csvFile:
Copy link
Member

Choose a reason for hiding this comment

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

Why not just open the readonly file groupingFile once at the start of this function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

better aproach because it closes the file automatically so it use less ressources.

Comment on lines +117 to +120
flagMappings = []
for entry in mappingFolderContent:
if entry.is_file() and entry.name[-4:] == ".csv":
flagMappings.append(entry.name[:-4])
Copy link
Member

Choose a reason for hiding this comment

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

Why does flagMappings have to be recomputed all the time?


blocktypes = ["blocks", "platforms"]

for blockType in blocktypes: # Roll over all Blocktypes
Copy link
Member

Choose a reason for hiding this comment

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

What is the goal of this loop? Could be simplified if I knew its goal...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

loop over the two blocktypes which we currrently have in our interlocking file (platforms + blocks), but also has the option to extend it, if we need.

Comment on lines +29 to +30
if int(number) == 6:
number = 0
Copy link
Member

Choose a reason for hiding this comment

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

Why this? numberToWord(6) would return "Six".

number %= 7

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

currently we dont have six as symbol


## Requirements
* Python3
* Configuration w for the model railway
Copy link
Member

Choose a reason for hiding this comment

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

"w"?

Where do you expect the files to be?

![](docs/rw5.png)

## How to get started
### Map the Signals with the Symbols
Copy link
Member

Choose a reason for hiding this comment

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

Where is your working directory located?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

its relative, so in the same folder like the markdown file - should be fine.

Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
Signed-off-by: Jochen Mehlich <[email protected]>
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.

3 participants