Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,4 @@ MEMORY_BANK.md
.DS_Store
scripts/
data/tmp.py
bounce.ps1
147 changes: 147 additions & 0 deletions AUTO_SYNC_CHANGES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
# BGS-Tally Improvements

## Changes Summary

This update includes two improvements:
1. **Auto-Sync RavenColonial** - Automatically enable RCSync when assigned to a project
2. **UI Worker Fix** - Prevent startup AttributeError when no system is loaded

---

## 1. Auto-Sync RavenColonial Feature

### Overview
This change automatically creates/tracks systems and enables RavenColonial sync (`RCSync`) when a commander docks at a construction ship or receives colonization data, **if**:
1. A project exists on RavenColonial for that system/market
2. The current commander is assigned to that project

**Key Improvement**: Systems are now **automatically added to BGS-Tally tracking** when you dock at a construction ship where you're assigned to a RavenColonial project - no manual system creation required!

### Changes Made

#### A. `bgstally/ravencolonial.py`
Added new method `check_auto_sync(system_address, market_id)`:
- Checks if a project exists via `/api/system/{systemAddress}/{marketId}`
- Verifies commander assignment via `/api/cmdr/{cmdr}`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You can get this information without making an additional call (/api/system/{systemAddress}/{marketId} includes a commanders section).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ahh thank you! I'm going to leave this unresolved so that I can remember to fix that if these changes intend to get implemented

- Handles both string buildIds and project objects from API
- Returns `True` if both conditions are met

**Location**: Lines 179-238

#### B. `bgstally/colonisation.py`
Added auto-sync checks in three locations:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this is a question for Aussi and the BGS-Tally users –– should we just assume RC integration for anyone who has a key setup or only use the integration if they enable it?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's always a good design principle to give maximum control to the users, and I've always tried to do this wherever possible in BGS-Tally.

Currently you can enable / disable synching with RC system-by-system. However, what is the actual benefit to a user of disabling RC sync for a system? If they have gone to the trouble of setting up RC with a key etc, would a user ever want to select a system to not be synched... I suspect the answer is no, in which case I don't see a problem with auto-synching. Happy to hear an opposing argument though, if there is a valid reason.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

My 2 cents: I think if a user has gone through the trouble to figure out where the RC API Key is in the first place, it's likely a safe assumption they will want that integration in BGS Tally. That being said, I'm curious why the API key is needed? As far as I can tell all of the API calls being made don't require the API key to be entered, but I may be missing something.


**1. ColonisationConstructionDepot Event (Lines 181-199)**
- **Auto-creates system** if it doesn't exist and RavenColonial project + assignment confirmed
- Enables RCSync for existing systems when conditions are met
- Prevents "Invalid ColonisationConstructionDepot event (no system)" warnings

**2. Docked Event - New System Creation (Lines 216-222)**
- Auto-enables RCSync when creating a new system during construction ship dock
- Checks project assignment before enabling sync

**3. Docked Event - Existing Systems (Lines 234-238)**
- Enables RCSync for already-tracked systems if not yet enabled

## API Endpoints Used

1. **Check Project**: `GET /api/system/{systemAddress}/{marketId}`
- Returns project data if it exists
- Returns 404 if no project found

2. **Check Commander Projects**: `GET /api/cmdr/{cmdr}`
- Returns list of projects the commander is assigned to

## Testing Steps

### Prerequisites
1. Have RavenColonial API key configured in BGS-Tally settings
2. Be assigned to a project on RavenColonial.com

### Test Case 1: Auto-create System (New System)
1. **Remove the system from BGS-Tally** if it exists (or use a fresh colonization system)
2. Dock at a construction ship where you're assigned to a RavenColonial project
3. **Expected**: System automatically added to BGS-Tally with RCSync enabled
4. **Expected Logs**:
- "Auto-creating system [system] with RCSync enabled"
- "Commander [cmdr] is assigned to project [buildId], enabling auto-sync"
5. **Verify**: System appears in BGS-Tally Colonisation window with sync icon (🔄 button visible)

### Test Case 2: Auto-enable for Existing System
1. Have a system already tracked in BGS-Tally with RCSync **disabled**
2. Dock at the construction ship or receive depot data
3. **Expected**: RCSync automatically enables
4. **Expected Log**: "Auto-enabling RCSync for [system]"
5. **Verify**: Contributions now sync to RavenColonial

### Test Case 3: No Auto-create (Not Assigned)
1. Dock at a construction ship where you're **NOT** assigned to the project
2. **Expected**: System is NOT created, warning logged
3. **Expected Log**: "Commander [name] is not assigned to project [id]"
4. **Expected**: "Invalid ColonisationConstructionDepot event (no system)" warning (normal behavior)

### Test Case 4: No Auto-create (No Project)
1. Dock at a construction ship with **no RavenColonial project**
2. **Expected**: System is NOT created
3. **Expected Logs**:
- "No project found for system [id], market [id]"
- "Invalid ColonisationConstructionDepot event (no system)" warning (normal behavior)

## Debug Logging

Watch for these log messages:

**Success Messages:**
- `"Auto-creating system {system} with RCSync enabled"` - New system auto-created
- `"Auto-enabling RCSync for newly created system {system}"` - RCSync enabled on dock
- `"Auto-enabling RCSync for {system}"` - RCSync enabled for existing system
- `"Commander {cmdr} is assigned to project {buildId}, enabling auto-sync"` - Assignment confirmed

**Info/Warning Messages:**
- `"No commander set, cannot check auto-sync"` - Commander name not loaded yet
- `"No project found for system {systemAddress}, market {marketId}"` - No RC project exists
- `"Commander {cmdr} is not assigned to project {buildId}"` - Not assigned to this project
- `"Error checking project: {status_code}"` - API error when checking project
- `"Error in check_auto_sync: {error}"` - Exception during auto-sync check

## Potential Issues

1. **API Key Required**: If no API key is configured, requests may fail
2. **Network Timeouts**: 5-second timeout on API calls may be too short in some cases
3. **Duplicate Checks**: Both dock and depot events may trigger - this is intentional for coverage
4. **Rate Limiting**: Multiple API calls per dock event - consider caching if this becomes an issue

---

## 2. UI Worker Startup Fix

### Overview
Fixed an AttributeError that occurred during EDMC startup when the UI worker tried to access system data before it was loaded.

### Changes Made

#### `bgstally/ui.py` (Line 491-493)
Added null check for `current_system` before accessing its attributes:
```python
# Fix: Check if current_system is not None before accessing its attributes
# This prevents AttributeError during startup when no system is loaded yet
if current_system is not None:
system_tick: str = current_system.get('TickTime')
```

### Error Fixed
```
AttributeError: 'NoneType' object has no attribute 'get'
at ui.py line 490: system_tick: str = current_system.get('TickTime')
```

**Impact**: Prevents harmless but annoying error message during EDMC startup.

---

## Future Improvements

1. Cache project/commander assignment to reduce API calls
2. Add UI notification when auto-sync is enabled
3. Allow users to opt-out of auto-sync via settings
4. Consider async API calls to avoid blocking
125 changes: 122 additions & 3 deletions bgstally/colonisation.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,27 @@ def journal_entry(self, cmdr, is_beta, sys, station, entry, state) -> None:
return

system = self.find_system({'StarSystem' : self.current_system, 'SystemAddress': self.system_id})
Debug.logger.debug(f"ColonisationContribution - system found: {system != None}, Hidden: {system.get('Hidden', True) if system else 'N/A'}, RCSync: {system.get('RCSync', False) if system else 'N/A'}")

if system != None and system.get('Hidden', True) == False and system.get('RCSync', False) == True:
found_progress = False
for progress in self.progress:
Debug.logger.debug(f"Checking progress: MarketID {progress.get('MarketID')} vs {self.market_id}, ProjectID: {progress.get('ProjectID')}")
if progress.get('MarketID', None) == self.market_id and progress.get('ProjectID', None) != None:
Debug.logger.info(f"Calling record_contribution for ProjectID {progress.get('ProjectID')}")
rc.record_contribution(progress.get('ProjectID', 0), entry.get('Contributions', []))
found_progress = True

# Just in case we don't have the ProjectID on the build, add it now.
b:dict|None = self.find_build(system, {'MarketID': self.market_id})
if b != None and b.get('ProjectID', None) == None:
self.modify_build(system, b.get('BuildID', ''), {'ProjectID': progress.get('ProjectID', None)})
break

if not found_progress:
Debug.logger.warning(f"No progress record found with ProjectID for MarketID {self.market_id}")
else:
Debug.logger.warning(f"ColonisationContribution skipped - conditions not met")

case 'ColonisationSystemClaim':
if not self.current_system or not self.system_id:
Expand All @@ -177,10 +188,96 @@ def journal_entry(self, cmdr, is_beta, sys, station, entry, state) -> None:
return

system = self.find_system({'StarSystem': self.current_system, 'SystemAddress' : self.system_id})

# If system doesn't exist, check if we should auto-create it based on RavenColonial assignment
if system == None:
Debug.logger.warning(f"Invalid ColonisationConstructionDepot event (no system): {entry}")
return
if self.system_id and self.market_id and self.current_system:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two things.

First, a ColonisationContribution cannot be done until after you've docked. Docking at a colonisation site will create the system & the build so doing it here is unnecessary. After docking you'll have ColonisationConstructionDepot events which will create the progress.

Second, BGS-Tally isn't just an RC client, it can function entirely without RC. This code assumes that if someone has a system in RC they want it synced with BGS-Tally. I chose to only sync with RC if the user specifically enables it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've been testing the existing release for a couple more hours this morning and feel that the automatic creation of RC projects is actually causing more problems than it solves. What I'm seeing is:

  • Autocreated project doesn't have a build type or body, and pulls the ugly $SYSTEMCOLONISATIONSHIP.... string for the station name. This could easily be parsed out, but you'd still have the issue of no build type or body
  • Autocreated project doesn't appear in the planning page for RC, even though the build exists
  • I could find no apparent way to create a build page based on an existing pre-planned site in RC, even though I have synced up my plan from RC
  • If you aren't the Architect and you try to deliver, the events are ignored by the plugin. There is no provision to create a project on behalf of the Architect. This is a feature that becomes important to groups working on large systems together

I believe changing the creation to a manual process would fix these issues and make for a more intuitive user experience, and also have a feeling that's why the original developer didn't make it automatic in SRVSurvey. How I think changing this to manual would resolve the issues:

  • Name can be pre-populated in the UI dialog and user can edit as desired. Build type and body would also be selectable.
  • I believe the above would resolve the issue of the build not appearing in the System page of RC
  • If a project page on RC needs created for a pre-planned RC site, this could be selected from a dropdown that is populated by the RC API as in my lightweight plugin or SRVSurvey. This also resolves the issue of the build page not appearing in the RC Systems page
  • Manual process would allow the assignment of a different CMDR name if the one delivering isn't the Architect. If they put in the wrong Architect name, it will error if the RC System is locked, otherwise it can be fixed later in RC by the Architect

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The workflow for project creation on a pre-planned site looks like this:

When the user hits the Create Project button, RC API sends the system body information and any preplanned sites to populate those fields in the UI. User selects their prebuilt site here:

image

And when they select the site, the Build Name, Construction Category, Model and Body are all filled in. They hit Create and are whisked away to the build page.
image

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

feel that the automatic creation of RC projects is actually causing more problems than it solves

Maybe... I really dislike making users enter information that the system already has. It's unnecessary workload and opens the possibility of user error resulting in duplication.

have a feeling that's why the original developer didn't make it automatic in SRVSurvey

From talking to him, originally projects & sites were entirely distinct and SRVSurvey was just a project client. This has been changing over time though when we last talked his feeling was that projects & sites still weren't properly connecting.

Autocreated project doesn't have a build type or body
Is it failing to match the build with the site, i.e. creating a new site? If so why? (This should be in the logs)
Generally speaking it should be able to match the colonisation site you landed at with the existing site [colonisation.py:199] in which case it would pick up the layout and body and fill these in automatically.

If you aren't the Architect and you try to deliver, the events are ignored by the plugin.
This should not be the case. What code is causing them to be ignored?

There is no provision to create a project on behalf of the Architect. This is a feature that becomes important to groups working on large systems together
This also should not be the case. If you land at a construction site a project should be created regardless of who the architect is. Is that not happening?

Manual process would allow the assignment of a different CMDR name if the one delivering isn't the Architect.
Assignment to what, the project or the system?

There is something that does need to be fixed which may address much of the above.

When RC added commander authentication (using FDev auth) it changed to allow open and closed systems. BGS-Tally currently only checks if the commander is the architect. It doesn't check if the commander has permission to edit the system even if they aren't the architect which it should.

project_id = RavenColonial(self).check_auto_sync(self.system_id, self.market_id)
if project_id:
Debug.logger.info(f"Auto-creating system {self.current_system} with RCSync enabled and ProjectID {project_id}")
system = self.find_or_create_system({'StarSystem': self.current_system, 'SystemAddress': self.system_id})
self.modify_system(system, {
'RCSync': True,
'Architect': self.cmdr,
'Hidden': False
})

# Also create the build for this construction depot
Debug.logger.info(f"Auto-creating build for market {self.market_id}")
build = self.find_or_create_build(system, {
'MarketID': self.market_id,
'Name': self.station if self.station else 'Construction Site',
'Body': self.body,
'State': BuildState.PROGRESS,
'Track': True,
'ProjectID': project_id
})
else:
Debug.logger.warning(f"Invalid ColonisationConstructionDepot event (no system): {entry}")
return
else:
Debug.logger.warning(f"Invalid ColonisationConstructionDepot event (no system): {entry}")
return

# Check if we should auto-enable RCSync for existing systems
project_id = None
if system.get('RCSync', False) == False and self.system_id and self.market_id:
project_id = RavenColonial(self).check_auto_sync(self.system_id, self.market_id)
if project_id:
Debug.logger.info(f"Auto-enabling RCSync for {system.get('StarSystem', 'Unknown')} with ProjectID {project_id}")
self.modify_system(system, {
'RCSync': True,
'Hidden': False
})

# Ensure Hidden=False for systems with RCSync enabled
if system.get('RCSync', False) == True and system.get('Hidden', True) == True:
Debug.logger.info(f"Setting Hidden=False for RCSync-enabled system {system.get('StarSystem', 'Unknown')}")
self.modify_system(system, {'Hidden': False})

# Ensure the build exists for this market
build = self.find_build(system, {'MarketID': self.market_id})
if build == None:
# Try to find by name if not found by MarketID
build = self.find_build(system, {'Name': self.station})
if build:
Debug.logger.info(f"Found build by name, adding MarketID {self.market_id}")
self.modify_build(system, build.get('BuildID', ''), {'MarketID': self.market_id})
else:
Debug.logger.info(f"Creating missing build for market {self.market_id}")
build = self.find_or_create_build(system, {
'MarketID': self.market_id,
'Name': self.station if self.station else 'Construction Site',
'Body': self.body,
'State': BuildState.PROGRESS,
'Track': True
})

# Ensure MarketID is set on the build
if build and build.get('MarketID', None) != self.market_id:
Debug.logger.info(f"Setting MarketID {self.market_id} on build {build.get('BuildID', 'unknown')}")
self.modify_build(system, build.get('BuildID', ''), {'MarketID': self.market_id})

# Ensure tracking is enabled
if build and build.get('Track', False) == False:
Debug.logger.info(f"Enabling tracking for build {build.get('BuildID', 'unknown')}")
self.modify_build(system, build.get('BuildID', ''), {'Track': True})
self.dirty = True
self.bgstally.ui.window_progress.update_display()

# Set ProjectID on build if we have it and it's missing
if build and project_id and build.get('ProjectID', None) == None:
Debug.logger.info(f"Setting ProjectID {project_id} on build {build.get('BuildID', 'unknown')}")
self.modify_build(system, build.get('BuildID', ''), {'ProjectID': project_id})

progress:dict = self.find_or_create_progress(self.market_id)

# Set ProjectID on progress record if we have it
if project_id and progress.get('ProjectID', None) != project_id:
Debug.logger.info(f"Setting ProjectID {project_id} on progress record for market {self.market_id}")
progress['ProjectID'] = project_id
self.dirty = True

if progress.get('ProjectID', None) != None and entry.get('ProjectID', None) == None:
entry['ProjectID'] = progress.get('ProjectID', None)
self.update_progress(self.market_id, entry)
Expand All @@ -195,7 +292,18 @@ def journal_entry(self, cmdr, is_beta, sys, station, entry, state) -> None:
# Colonisation ship is always the first build. Construction site can be any build
if '$EXT_PANEL_ColonisationShip' in f"{self.station}" or 'Construction Site' in f"{self.station}":
Debug.logger.debug(f"Docked at construction site. Finding/creating system and build")
if system == None: system = self.find_or_create_system({'StarSystem': self.current_system, 'SystemAddress' : self.system_id})
if system == None:
system = self.find_or_create_system({'StarSystem': self.current_system, 'SystemAddress' : self.system_id})
# Check if we should auto-enable RCSync for newly created system
if self.system_id and self.market_id:
project_id = RavenColonial(self).check_auto_sync(self.system_id, self.market_id)
if project_id:
Debug.logger.info(f"Auto-enabling RCSync for newly created system {self.current_system} with ProjectID {project_id}")
self.modify_system(system, {
'RCSync': True,
'Architect': self.cmdr,
'Hidden': False
})
build = self.find_or_create_build(system, {'MarketID': self.market_id, 'Name': self.station, 'Body': self.body})
build_state = BuildState.PROGRESS
# Complete station so find it and add/update as appropriate.
Expand All @@ -208,6 +316,17 @@ def journal_entry(self, cmdr, is_beta, sys, station, entry, state) -> None:
if system == None or build == None:
return

# Check if we should auto-enable RCSync when docking at construction ship
if '$EXT_PANEL_ColonisationShip' in f"{self.station}" or 'Construction Site' in f"{self.station}":
if system.get('RCSync', False) == False and self.system_id and self.market_id:
project_id = RavenColonial(self).check_auto_sync(self.system_id, self.market_id)
if project_id:
Debug.logger.info(f"Auto-enabling RCSync for {system.get('StarSystem', 'Unknown')} on dock")
self.modify_system(system, {
'RCSync': True,
'Hidden': False
})

# Update the system details
if system.get('Name', None) == None: system['Name'] = self.current_system

Expand Down
Loading
Loading