Skip to content

refactor: migrate MainLoop to TypeScript#2675

Open
HoloTheDrunk wants to merge 10 commits intoiTowns:masterfrom
HoloTheDrunk:refactor/typescript-mainloop
Open

refactor: migrate MainLoop to TypeScript#2675
HoloTheDrunk wants to merge 10 commits intoiTowns:masterfrom
HoloTheDrunk:refactor/typescript-mainloop

Conversation

@HoloTheDrunk
Copy link
Contributor

@HoloTheDrunk HoloTheDrunk commented Jan 20, 2026

Description

Migrates MainLoop to TypeScript and cleans up some of the outdated and/or unused code.

Motivation and Context

Part of the project-wide migration to TypeScript, I needed this file's migration to happen for other PRs I'm working on.

@Desplandis
Copy link
Contributor

I did some brief explanation of the main loop mechanism in ##2630 if that helps!

@HoloTheDrunk HoloTheDrunk marked this pull request as ready for review February 11, 2026 17:28
@HoloTheDrunk HoloTheDrunk force-pushed the refactor/typescript-mainloop branch from 4313dec to f148e62 Compare February 11, 2026 17:29
Copy link
Contributor

@Desplandis Desplandis left a comment

Choose a reason for hiding this comment

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

I have some comments but +1 for typechecking this module + simplify sub.elements mess.

AFTER_RENDER: 'after_render',
/** Fired at the end of the update */
UPDATE_END: 'update_end',
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Since they are used as a constant set of events, you could refine the type by marking it as as const. As such the inferred type would be

{
    UPDATE_START: 'update_start';
    ...
}

instead of

{
    UPDATE_START: string;
    ...
}

Copy link
Contributor

@Desplandis Desplandis Feb 24, 2026

Choose a reason for hiding this comment

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

You did not take my comment into account.

Suggested change
};
} as const;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I distinctly remember making that change, not sure what happened...


// Request redraw
view.notifyChange(true);
view.notifyChange({});
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer calling it without parameters or with layer or view as parameter if a layer or a set of layers have been updated!

@Desplandis
Copy link
Contributor

@HoloTheDrunk One of my comment was not resolved

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.

3 participants