Skip to content

[WIP] Add error handling and logging for TaxProductionUISystem - #2

Merged
binded-zz merged 1 commit into
mainfrom
copilot/add-error-handling-to-ui-system
Mar 29, 2026
Merged

[WIP] Add error handling and logging for TaxProductionUISystem#2
binded-zz merged 1 commit into
mainfrom
copilot/add-error-handling-to-ui-system

Conversation

Copilot AI commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Original prompt

This PR implements essential stabilization and best practices for Cities: Skylines II mods:

  1. TaxProductionUISystem.cs:

    • Adds try/catch error handling and robust logging to all UI trigger actions.
    • Uses wrapper functions for safe trigger bindings (prevents crashes from unhandled exceptions).
    • Preserves all Colossal API interfaces (extends UISystemBase as required by the modding API).
    • Keeps all logic; just adds error-safety and better debugging.
    • Example safe wrapper methods added.
  2. TaxProductionWindow.css:

    • Ensures .tax-production-window style includes display: block; after position: fixed; so the window always becomes visible when triggered.

Result: The mod builds cleanly, is robust against runtime UI errors, user toggles, and looks/works as intended.

The following is the prior conversation context from the user's chat exploration (may be truncated):

Assistant: I've identified and fixed the problem! The windows aren't opening because the CSS is missing the display property, so even when the window's visibility state changes to true, the element stays hidden.

The Issue

In src/UI/components/TaxProductionWindow.css, the .tax-production-window class doesn't have a display property. Without it, the element defaults to display: none, making it invisible regardless of the React conditional rendering.

The Fix

I've updated the CSS to add:

display: block;

This ensures:

  • ✅ The window renders and is visible when isVisible is true
  • ✅ The close button works to hide it again
  • ✅ The settings panel also displays correctly
  • ✅ The toolbar button's click handler properly toggles the window

The C# bindings (TaxProductionUISystem.cs) and React components (TaxMod.tsx, TaxWindow.tsx) are working correctly - they were properly toggling the visibility state. The CSS was just preventing the DOM element from being displayed.

Test it now: Click the toolbar button (tax icon in the top-left) to toggle the window open and closed!

User: look at https://github.com/thiago-rcarvalho/cs2-parking-fee-control

Assistant: This is an excellent reference implementation! The Parking Fee Control mod shows best practices for Cities Skylines 2 modding with proper C# and TypeScript/React integration. Here are the key lessons that apply to your project:

Key Architecture Patterns from the Reference

1. Proper C# System Structure (Mod.cs)

  • ✅ Clear module entry point with logging
  • ✅ Loads configuration from persistent storage (JSON config)
  • ✅ Registers both gameplay systems and UI systems in OnLoad()
  • ✅ Proper error handling with try-catch blocks

2. Comprehensive UISystem (ParkingFeeUISystem.cs)

  • Uses Value Bindings for two-way data sync (like your implementation)
  • Uses Trigger Bindings for events (button clicks, updates)
  • Implements IJsonWritable/IJsonReadable for structured data serialization
  • Properly manages entity/component interactions with the game world
  • Has comprehensive logging for debugging

3. UI System Pattern - Key Methods

// Bindings setup in OnCreate()
AddBinding(new ValueBinding<DataType>("groupName", "propertyName", _data));
AddBinding(new TriggerBinding<UpdateType>("groupName", "triggerName", UpdateMethod));

// Update methods receive data from UI
private void UpdateMethod(UpdateType update) { ... }

// Notify UI of changes
_binding.Update(newData);

What Your Project Should Adopt

Problem in Your Code:

Your CSS is missing the display property - that's already fixed, but you should also:

  1. Add Proper Trigger Bindings - Your C# system needs to receive updates from the UI
  2. Use Value Bindings Correctly - Make sure data flows both ways
  3. Implement Error Handling - Wrap operations in try-catch

Fix Your TaxProductionUISystem.cs

Update it to be more like the reference implementation:

using Colossal.UI.Binding;
using Game.UI;
using System;

namespace CitiesSkylines2Mod
{
    public partial class TaxProductionUISystem : UISystemBase
    {
        private const string Group = "taxProduction";
        
        private bool m_IsVisible = false;
        private bool m_SettingsVisible = false;
        private float m_TaxRate = 0f;
        private bool m_ButtonEnabled = true;

        private ValueBinding<bool> _isVisibleBinding;
        private ValueBinding<bool> _settingsVisibleBinding;
        private ValueBinding<float> _taxRateBinding;
        private ValueBinding<bool> _buttonEnabledBinding;

        protected override void OnCreate()
        {
            base.OnCreate();

            var settings = ModEntry.Settings;
            m_ButtonEnabled = settings?.ButtonEnabled ?? true;
            m_TaxRate = settings?.DefaultTaxRate ?? 15;

            // Add bindings for data flowing FROM C# TO UI
            AddBinding(_isVisibleBinding = new ValueBinding<bool>(Group, "isVisible", m_IsVisible));
            AddBinding(_...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

📱 Kick off Copilot coding agent tasks wherever you are with [GitHub Mobile](https://gh.io/cca-mobile-docs), available on iOS and Android.

@binded-zz
binded-zz marked this pull request as ready for review March 29, 2026 03:40
@binded-zz
binded-zz merged commit 797af14 into main Mar 29, 2026
1 check failed
Copilot stopped work on behalf of binded-zz due to an error March 29, 2026 03:41
Copilot AI requested a review from binded-zz March 29, 2026 03:41
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.

2 participants