[tryuan99]

This PR refactors common simulation-related constants and utilities into separate files for better reusability. In particular, all simulation-related constants, including the agent types, the event types, and the log file prefixes, are now defined in constants.py while utility functions, including finding the latest telemetry file and event log, are now defined in utils.py.

[stacklane-pr-stack-visualizer]

🧱 Stack PR · Base of stack

Stack Structure:

• [Tools] Refactor common simulation-related constants and utilities #160 titan/visualize_sim_constants ← YOU ARE HERE

[coderabbitai]

Actionable comments posted: 5

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c304d4 and 9b19007.

📒 Files selected for processing (4)

• Tools/constants.py (1 hunks)
• Tools/unity_utils.py (1 hunks)
• Tools/utils.py (1 hunks)
• Tools/visualize_log.py (6 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

Tools/visualize_log.py (3)

Tools/constants.py (2)

• AgentType (6-9)
• EventType (12-17)

Tools/utils.py (4)

• find_latest_telemetry_file (63-69)
• find_latest_event_log (72-78)
• read_telemetry_file (100-109)
• read_event_log (112-124)

Tools/unity_utils.py (1)

• get_persistent_data_directory (135-147)

🪛 Ruff (0.14.8)

Tools/unity_utils.py

127-128: Avoid specifying long messages outside the exception class

(TRY003)

---

127-127: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

128-128: f-string without any placeholders

Remove extraneous f prefix

(F541)

Tools/visualize_log.py

151-151: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Test plugins on ubuntu-latest
• GitHub Check: Test plugins on macos-latest
• GitHub Check: Test plugins on windows-latest

🔇 Additional comments (7)

Tools/visualize_log.py (3)

18-38: LGTM - Well-structured visualization configuration.

The EventMarker dataclass and the mapping dictionaries provide clean, centralized configuration for visualization styles.

---

65-66: LGTM - Clean enum-based event filtering.

Using EventType enum constants for comparisons improves type safety and readability.

---

108-119: LGTM - Clean iteration over event markers.

The refactored plotting logic using EVENT_TYPE_TO_MARKER is clear and maintainable.

Tools/utils.py (3)

45-60: LGTM - Correct handling of empty results and modification time.

The function properly returns None when no files match and uses st_mtime for cross-platform consistency.

---

81-97: LGTM - Subdirectory discovery implementation is correct.

The generator is properly consumed in the list comprehension with the is_dir() filter.

---

100-123: LGTM - Clean data reading utilities.

The event log sanitization (uppercase + strip) ensures consistent comparison with EventType enum values.

Tools/constants.py (1)

1-24: LGTM - Clean enum definitions and constants.

The enums and constants are well-structured. Both TELEMETRY_FILE_PREFIX and EVENT_LOG_FILE_PREFIX are used in Tools/utils.py, and the enum types are used in Tools/visualize_log.py. Note that StrEnum requires Python 3.11+.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

LGTM - Consider returning Path for consistency.

The platform-specific logic is correct. Minor suggestion: the rest of this file uses Path objects, but this function returns str. Consider returning Path for consistency, though this is optional.

-def get_persistent_data_directory() -> str:
+def get_persistent_data_directory() -> Path:
     """Returns the path to Unity's persistent data directory."""
     system = platform.system()
     if system == "Windows":
-        return os.path.expandvars(
-            r"%USERPROFILE%\AppData\LocalLow\BAMLAB\micromissiles\Logs")
+        return Path(os.path.expandvars(
+            r"%USERPROFILE%\AppData\LocalLow\BAMLAB\micromissiles\Logs"))
     elif system == "Darwin":
-        return os.path.expanduser(
-            "~/Library/Application Support/BAMLAB/micromissiles/Logs")
+        return Path(
+            "~/Library/Application Support/BAMLAB/micromissiles/Logs").expanduser()
     elif system == "Linux":
-        return os.path.expanduser("~/.config/unity3d/BAMLAB/micromissiles/Logs")
+        return Path("~/.config/unity3d/BAMLAB/micromissiles/Logs").expanduser()
     else:
         raise NotImplementedError(f"Unsupported platform: {system}.")

🤖 Prompt for AI Agents

In Tools/unity_utils.py around lines 135 to 146 the function
get_persistent_data_directory() currently returns a str while the rest of the
module uses Path objects; change the return type to pathlib.Path and construct
and return Path objects instead of raw strings (convert expanded/expandeduser
results with Path(...) or use Path.home()/os-specific Path joins), update the
function signature to -> Path and ensure pathlib.Path is imported at the top of
the file.

[daniellovell]

LGTM 👍