From 40f5a6ab7cd1a0a30dc1742580d2492435952226 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 18:02:21 +0100 Subject: [PATCH 01/11] Fix timestamp not set on clearing instance saves Set the save's updatedAtMs timestamp when clearing the save list of a reassigned or deleted instance. This fixes the Web UI not updating the saves list to remove the saves when an instance is reassigned. --- packages/controller/src/Controller.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 3e81c0c1..d9a1332b 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -876,9 +876,11 @@ export default class Controller { */ clearSavesOfInstance(instanceId: number) { const updates = []; + const now = Date.now(); for (const [id, save] of this.saves) { if (save.instanceId === instanceId) { save.isDeleted = true; + save.updatedAtMs = now; updates.push(save); this.saves.delete(id); } From 7ba14f74330dbf13602b8b061629f9ee6db991ac Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 18:12:30 +0100 Subject: [PATCH 02/11] Fix instance details not updating on reassigmnent Always send the instance details update when an instance is reassigned, not just when it is unassigned. The idea for not sending an update when assigned to a host was that the host would send an update on the status that in turn updates the instance details. However this is not actually the case; when a stopped instance is reassigned its status does not change so the update is ignored. --- CHANGELOG.md | 1 + packages/controller/src/Controller.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68e772fe..9a507824 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fixed an unhandled error in Subspace Storage crashing the host. - Fixed an unhandled rejection in disconnect logic crashing the host. +- Fixed instance screen in Web UI not updating when an instance was reassigned. ## Version 2.0.0-alpha.16 diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index d9a1332b..5422fedf 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -941,9 +941,9 @@ export default class Controller { ); } else { instance.status = "unassigned"; - instance.updatedAtMs = Date.now(); - this.instanceDetailsUpdated([instance]); } + instance.updatedAtMs = Date.now(); + this.instanceDetailsUpdated([instance]); } /** From 021af793fcece14d4e96a4eba279f2d663c0210f Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 18:13:57 +0100 Subject: [PATCH 03/11] Handle instance status updates with port change If an instance is reassigned its port may change without the status changing, these updates should not be ignored. --- packages/controller/src/HostConnection.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/controller/src/HostConnection.ts b/packages/controller/src/HostConnection.ts index 564b5b96..ab566a40 100644 --- a/packages/controller/src/HostConnection.ts +++ b/packages/controller/src/HostConnection.ts @@ -178,7 +178,10 @@ export default class HostConnection extends BaseConnection { // changed. Hosts also send status updates on assignInstance which // for hacky reason is also used to push config changes and // restablish status after a connection loss. - if (instance.status === request.status) { + if ( + instance.status === request.status + && instance.gamePort === request.gamePort + ) { return; } From 2e25831b4df2ce9bd2e8dc5cd5bbe3c04cf4c91b Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 18:16:15 +0100 Subject: [PATCH 04/11] Warn on subscription updates with same timestamp If the value in a subscribed list is updated while the updatedAtMs timestamp is kept the same then the caching logic of the subscription system will cause updates to be silently dropped. Log a warning if a subscriber catches this happening to help tracking down such cases. --- packages/lib/src/subscriptions.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/lib/src/subscriptions.ts b/packages/lib/src/subscriptions.ts index de155090..120cf2fe 100644 --- a/packages/lib/src/subscriptions.ts +++ b/packages/lib/src/subscriptions.ts @@ -2,6 +2,7 @@ import { Type, Static } from "@sinclair/typebox"; import { Link, Event, EventClass, RequestHandler, WebSocketClientConnector, WebSocketBaseConnector } from "./link"; import { logger } from "./logging"; import { Address, MessageRequest, IControllerUser } from "./data"; +import isDeepStrictEqual from "./is_deep_strict_equal"; export type SubscriptionRequestHandler = RequestHandler | null>; export type EventSubscriberCallback = (updates: T[], synced: boolean) => void @@ -204,6 +205,14 @@ export class EventSubscriber< async _handle(event: EventSubscribable) { for (const value of event.updates) { this.lastResponseTimeMs = Math.max(this.lastResponseTimeMs, value.updatedAtMs); + // Warn about updates which changes content but don't update updatedAtMs + const existing = this.values.get(value.id as K); + if (existing && existing.updatedAtMs === value.updatedAtMs && !isDeepStrictEqual(existing, value)) { + logger.warn( + `${this.Event.name} contains update for value with id ${value.id} that has an identical ` + + "updatedAtMs timestamp but differing content" + ); + } if (value.isDeleted) { this.values.delete(value.id as K); } else { From 6b85567bc63135e5de105d7d83234dbba40e04a4 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 18:45:06 +0100 Subject: [PATCH 05/11] Provide theme to top level ErrorBoundary If the theme is not provided then the App's ErrorBoundary will render white text on white background, and vice versa for black. --- packages/web_ui/src/components/App.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/web_ui/src/components/App.tsx b/packages/web_ui/src/components/App.tsx index 02e9c726..e4dcfdc6 100644 --- a/packages/web_ui/src/components/App.tsx +++ b/packages/web_ui/src/components/App.tsx @@ -104,16 +104,16 @@ export default function App(props: AppProps) { } return ( - - + + {page} - - + + ); } From 2df67e32daf18676c11d1b18032cd6f1f8614399 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 18:46:55 +0100 Subject: [PATCH 06/11] Set key for top level page fragment Without a unique per page key set react will try to reuse the existing state and components of the previous page and update them to match the new page. This causes unexpected behaviour as components get turned into into a Frankenstein abomination of the previous page and the current one fused into one. Fix by setting a key on the top level fragment of the page which in turn forces React to re-render the whole page whenever the user switches to another page. --- CHANGELOG.md | 1 + packages/web_ui/src/components/SiteLayout.tsx | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a507824..fec0adec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Fixed an unhandled error in Subspace Storage crashing the host. - Fixed an unhandled rejection in disconnect logic crashing the host. - Fixed instance screen in Web UI not updating when an instance was reassigned. +- Fixed an error thrown in the rendering of one page in the Web UI causing all pages to become inaccessible. ## Version 2.0.0-alpha.16 diff --git a/packages/web_ui/src/components/SiteLayout.tsx b/packages/web_ui/src/components/SiteLayout.tsx index 2bf69d47..07a995d1 100644 --- a/packages/web_ui/src/components/SiteLayout.tsx +++ b/packages/web_ui/src/components/SiteLayout.tsx @@ -1,4 +1,4 @@ -import React, { useContext, useEffect, useState } from "react"; +import React, { Fragment, useContext, useEffect, useState } from "react"; import { Route, Routes, useNavigate } from "react-router-dom"; import { Dropdown, Layout, Menu, MenuProps } from "antd"; import UserOutlined from "@ant-design/icons/UserOutlined"; @@ -115,12 +115,12 @@ export default function SiteLayout() { {combinedPages.map(({path, sidebarPath, content}) => + element={ {content} - } + } />)} } /> From 535e43b5278b9e5c1229bc798e9132e71c60b6f7 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 19:20:33 +0100 Subject: [PATCH 07/11] Rename save.list.subscribe permission Follow the same pattern as all the other subscribable lists by having a separate .list and .subscribe permissions for saves. This also fixes the save list not being visible due to a typo in the permission in the Web UI. --- CHANGELOG.md | 1 + packages/lib/src/data/Role.ts | 10 +++++++++- packages/lib/src/data/messages_instance.ts | 2 +- packages/lib/src/permissions.ts | 6 +++--- packages/web_ui/src/components/InstanceViewPage.tsx | 2 +- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fec0adec..b5a6b7b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Fixed an unhandled rejection in disconnect logic crashing the host. - Fixed instance screen in Web UI not updating when an instance was reassigned. - Fixed an error thrown in the rendering of one page in the Web UI causing all pages to become inaccessible. +- Fixed Web UI not showing instance saves when the permission to list and subscribe the saves is granted but the user is not a cluster admin. ## Version 2.0.0-alpha.16 diff --git a/packages/lib/src/data/Role.ts b/packages/lib/src/data/Role.ts index 6356def8..56f104bc 100644 --- a/packages/lib/src/data/Role.ts +++ b/packages/lib/src/data/Role.ts @@ -19,12 +19,20 @@ export default class Role { permissions: Type.Array(Type.String()), }); + static remapPermission(permission: string) { + // migrate: core.instance.save.list.subscribe was renamed in alpha 17 + if (permission === "core.instance.save.list.subscribe") { + return "core.instance.save.subscribe"; + } + return permission; + } + static fromJSON(json: Static) { return new this( json.id, json.name, json.description, - new Set(json.permissions), + new Set(json.permissions.map(this.remapPermission)), ); } diff --git a/packages/lib/src/data/messages_instance.ts b/packages/lib/src/data/messages_instance.ts index f71b2a13..2a37d086 100644 --- a/packages/lib/src/data/messages_instance.ts +++ b/packages/lib/src/data/messages_instance.ts @@ -361,7 +361,7 @@ export class InstanceSaveDetailsUpdatesEvent { static type = "event" as const; static src = ["instance", "host", "controller"] as const; static dst = ["controller", "control"] as const; - static permission = "core.instance.save.list.subscribe" as const; + static permission = "core.instance.save.subscribe" as const; constructor( public updates: SaveDetails[], diff --git a/packages/lib/src/permissions.ts b/packages/lib/src/permissions.ts index cbaf9a93..306047de 100644 --- a/packages/lib/src/permissions.ts +++ b/packages/lib/src/permissions.ts @@ -239,11 +239,11 @@ definePermission({ definePermission({ name: "core.instance.save.list", title: "List saves", - description: "List the saves currently on the instance.", + description: "List the saves currently on instances.", }); definePermission({ - name: "core.instance.save.list.subscribe", - title: "Subscribe to save list updatess", + name: "core.instance.save.subscribe", + title: "Subscribe to save updates", description: "Subscribe to be notifed on updates to the list of saves of instances.", }); definePermission({ diff --git a/packages/web_ui/src/components/InstanceViewPage.tsx b/packages/web_ui/src/components/InstanceViewPage.tsx index 13258761..f5b7281c 100644 --- a/packages/web_ui/src/components/InstanceViewPage.tsx +++ b/packages/web_ui/src/components/InstanceViewPage.tsx @@ -204,7 +204,7 @@ export default function InstanceViewPage() { { - account.hasAllPermission("core.instance.save.list", "core.instance.save.list_subscribe") + account.hasAllPermission("core.instance.save.list", "core.instance.save.subscribe") && } { From d3f8c993b16e97829272ea2f21f4a4d2497cf4c0 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 19:26:23 +0100 Subject: [PATCH 08/11] Fix bad permission checks in Web UI Fix attempts to check for permissions that don't exist in the Web UI that has gone unnoticed because the check still succeeds when the account has the cluster admin permission. --- CHANGELOG.md | 1 + packages/web_ui/src/components/HostViewPage.tsx | 2 +- packages/web_ui/src/components/ModPackViewPage.tsx | 2 +- packages/web_ui/src/components/ModsPage.tsx | 4 ++-- packages/web_ui/src/pages.tsx | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5a6b7b5..65cbac3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Fixed instance screen in Web UI not updating when an instance was reassigned. - Fixed an error thrown in the rendering of one page in the Web UI causing all pages to become inaccessible. - Fixed Web UI not showing instance saves when the permission to list and subscribe the saves is granted but the user is not a cluster admin. +- Similarly fixed Web UI not showing the mod pack create, delete, list and host revoke token actions if the user is not a cluster admin. ## Version 2.0.0-alpha.16 diff --git a/packages/web_ui/src/components/HostViewPage.tsx b/packages/web_ui/src/components/HostViewPage.tsx index fc71311d..f70d9e2c 100644 --- a/packages/web_ui/src/components/HostViewPage.tsx +++ b/packages/web_ui/src/components/HostViewPage.tsx @@ -48,7 +48,7 @@ export default function HostViewPage() { let hostButtons = { - account.hasPermission("core.host.revoke_access") + account.hasPermission("core.host.revoke_token") && - {account.hasPermission("core.mod-pack.delete") && - {account.hasPermission("core.mod-pack.create") && } - {account.hasPermission("core.mod-pack.create") && } + {account.hasPermission("core.mod_pack.create") && } + {account.hasPermission("core.mod_pack.create") && } } /> account.hasAnyPermission("core.mod.list", "core.mod-pack.list"), + permission: (account: UserAccount) => account.hasAnyPermission("core.mod.list", "core.mod_pack.list"), content: , }, { From 44b0a8ff834434c6e3cd75ed232a3e24526bfb5a Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 19:28:21 +0100 Subject: [PATCH 09/11] Throw error on bad permission check in Wed UI Throw an error if an attempt is made to check for a permission that does not exist in the Web UI to prevent future bad permission checks going unnoticed. --- packages/web_ui/src/model/account.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/web_ui/src/model/account.ts b/packages/web_ui/src/model/account.ts index 8086bf3a..dfce6832 100644 --- a/packages/web_ui/src/model/account.ts +++ b/packages/web_ui/src/model/account.ts @@ -22,6 +22,12 @@ export function useAccount(): UserAccount { }; }, [control]); + function checkPermissionExists(permission: string) { + if (!lib.permissions.has(permission)) { + throw new Error(`permission ${permission} does not exist`); + } + } + function hasPermission(permission: string): boolean { for (let role of roles) { if (role.permissions.includes("core.admin") || role.permissions.includes(permission)) { @@ -38,12 +44,16 @@ export function useAccount(): UserAccount { if (!roles) { return null; } + checkPermissionExists(permission); return hasPermission(permission); }, hasAnyPermission(...permissions: string[]): boolean | null { if (!roles) { return null; } + for (let permission of permissions) { + checkPermissionExists(permission); + } for (let permission of permissions) { if (hasPermission(permission)) { return true; @@ -55,6 +65,9 @@ export function useAccount(): UserAccount { if (!roles) { return null; } + for (let permission of permissions) { + checkPermissionExists(permission); + } for (let permission of permissions) { if (!hasPermission(permission)) { return false; From 6bf2bf51f23560570ee9b0d93bfa5eced165fc4a Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 5 Feb 2024 19:29:50 +0100 Subject: [PATCH 10/11] Remove deprecated showArrow prop Ant design has deprecated the Select's showArrow prop. It's now always shown by default. --- packages/web_ui/src/components/UserViewPage.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/web_ui/src/components/UserViewPage.tsx b/packages/web_ui/src/components/UserViewPage.tsx index b0d138a4..8770571f 100644 --- a/packages/web_ui/src/components/UserViewPage.tsx +++ b/packages/web_ui/src/components/UserViewPage.tsx @@ -88,7 +88,6 @@ export default function UserViewPage() { onChange={() => setRolesDirty(true)} loading={!roles} mode="multiple" - showArrow={true} filterOption={(inputValue:string, option: any): boolean => { let role = roles.get(option.value); return role?.name.toLowerCase().includes(inputValue.toLowerCase()) ?? false; From dcf216ca09ba8c7c37da75053aa82a7ff58845b9 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 6 Feb 2024 20:50:19 +0100 Subject: [PATCH 11/11] Fix research sync applying progress to wrong level When the level of a technology changes either by setting level directly or by setting researched to true, the saved progress of the research is reset only if the technology is not the current research. If during the initial synchronisation with the controller an instance has progressed on a multi level research, it's the current research, and the controller's level of that research is higher, then the level is set to the higher level and the game keeps the progress. This in turn results in all of the progress made on the previous level being added to the higher level. Fix by setting the progress to zero when changing the level or researched status of a technology that's the active research. --- CHANGELOG.md | 1 + plugins/research_sync/module/sync.lua | 13 ++++++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65cbac3b..77598f4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Fixed an error thrown in the rendering of one page in the Web UI causing all pages to become inaccessible. - Fixed Web UI not showing instance saves when the permission to list and subscribe the saves is granted but the user is not a cluster admin. - Similarly fixed Web UI not showing the mod pack create, delete, list and host revoke token actions if the user is not a cluster admin. +- Fixed Research Sync applying progress of a previous techology level to the level on the controller when it's the active research during instance startup. ## Version 2.0.0-alpha.16 diff --git a/plugins/research_sync/module/sync.lua b/plugins/research_sync/module/sync.lua index 03a1bb9e..7abe268a 100644 --- a/plugins/research_sync/module/sync.lua +++ b/plugins/research_sync/module/sync.lua @@ -137,10 +137,21 @@ function research_sync.sync_technologies(data) for _, tech_data in pairs(game.json_to_table(data)) do local tech = force.technologies[tech_data[nameIndex]] if tech and tech.level <= tech_data[levelIndex] then - tech.level = math.min(tech_data[levelIndex], tech.prototype.max_level) + local new_level = math.min(tech_data[levelIndex], tech.prototype.max_level) + if new_level ~= tech.level then + -- when the level of the current research changes the + -- progress is not automatically reset. + if force.current_research == tech then + force.research_progress = 0 + end + tech.level = new_level + end local progress if tech_data[researchedIndex] then + if force.current_research == tech then + force.research_progress = 0 + end tech.researched = true progress = nil elseif tech_data[progressIndex] then