Skip to content

Conversation

@tomarnepedersen
Copy link
Collaborator

@tomarnepedersen tomarnepedersen commented Sep 24, 2025

Check that documentation is working, and that the target ship numbering is ok.

This PR resolves #68

@StephanieKemna StephanieKemna removed their assignment Oct 3, 2025
@StephanieKemna StephanieKemna added the bug Something isn't working label Oct 3, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses ship ID numbering issues and improves documentation with several key improvements to the traffic generation system.

  • Fixed ship ID assignments to ensure unique IDs across own ship and target ships
  • Enhanced documentation with better output file descriptions and navigation links
  • Updated version numbers and CHANGELOG entries to reflect the bug fixes

Reviewed Changes

Copilot reviewed 66 out of 68 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/trafficgen/read_files.py Fixed ship ID defaults from 0 to 1 for own ship and improved target ship ID calculation
src/trafficgen/encounter.py Updated target ship ID assignment logic to be relative to own ship ID
src/trafficgen/cli.py Removed duplicate verbosity option and improved type annotation
docs/source/output_files.rst Added new comprehensive documentation for output file format
docs/source/usage.rst Enhanced usage documentation with better organization and cross-references
pyproject.toml Updated version from 0.8.1 to 0.8.2
Multiple JSON files Updated ship IDs and version numbers in generated traffic situations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

StephanieKemna and others added 2 commits October 3, 2025 10:04
Ownship ID is now 1, so update example output.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Target ship IDs now start at 2, update example.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@StephanieKemna
Copy link
Collaborator

@tomarnepedersen It seems that if I specify an ID for the target ships in the target ship .json config files, they get ignored. I tried by adding an ID (random nr, 25) in target_ship_2.json.
I reckon it should be possible to do that though - someone's test/simulation system might have some assumptions on target ship IDs that we can then adhere to (e.g. own vessels have IDs 1-99, target ships have IDs 100-x).
I can do another quick test in another repo where I use STG to confirm - I am sure we specified IDs for target ships there.
I confirmed that I can change the ID of the ownship - but the tgt ships are only increments on top of that - it ignores the IDs I am setting in config. It should only do that if I am not setting an ID, I reckon (though note that if some IDs are set but not all, you will need to check that you do not conflict with that..)

@tomarnepedersen
Copy link
Collaborator Author

it is correct that target ship IDs are ignored at the moment. The reason is that same target ship may be used several times, and then they will have the same ID. I haven't used time to figure out a solution for this.

Copy link
Collaborator

@StephanieKemna StephanieKemna left a comment

Choose a reason for hiding this comment

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

Comments were directly in PR - discussed here and in person, oddities fixed, so looking good now!

@tomarnepedersen tomarnepedersen merged commit 24f75c0 into main Oct 6, 2025
8 checks passed
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

None yet

Development

Successfully merging this pull request may close these issues.

TS id numbers are same in output situation files

2 participants