Skip to content

Merge readers and writers#516

Merged
lahm86 merged 9 commits intoLostArtefacts:masterfrom
lahm86:issue-507-levelio-stage3
Aug 7, 2023
Merged

Merge readers and writers#516
lahm86 merged 9 commits intoLostArtefacts:masterfrom
lahm86:issue-507-levelio-stage3

Conversation

@lahm86
Copy link
Copy Markdown
Collaborator

@lahm86 lahm86 commented Aug 7, 2023

Each TRXLevelReader and TRXLevelWriter has been merged into TRXLevelControl. The writing logic still remains within the level classes themselves, the next stage will involve separating out common code and further streamlining the control classes. Ultimately, the level classes will primarily be data structures and all IO logic handled in the control classes.

One gotcha was with multithreading in the randomizer itself (AbstractLevelProcessor and children) - the locks here are now shared instead of one for reading and another for writing.

Additionally, tests now use memory streams instead of writing to disk.

Part of #507.

@lahm86 lahm86 added the internal label Aug 7, 2023
@lahm86 lahm86 added this to the 1.8.0 milestone Aug 7, 2023
@lahm86 lahm86 self-assigned this Aug 7, 2023
@lahm86 lahm86 requested review from chreden, makotocchi and rr- August 7, 2023 17:35
Comment thread LocationExport/Program.cs Outdated
_reader1 = new TR1LevelReader();
_reader3 = new TR3LevelReader();
_reader1 = new TR1LevelControl();
_reader3 = new TR3LevelControl();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't it enough to call new()?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I missed quite a few of these, so updated now.

We have lots of messages like this since migrating to .NET Core, I should aim to fix these while editing the affected files. Ideally this number will eventually become 0.

image

Copy link
Copy Markdown

@rr- rr- Aug 7, 2023

Choose a reason for hiding this comment

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

Maybe this could be tackled soon so that the PRs / CRs don't get polluted with stylistic comments?
Also there's the noise from the GH Actions bot.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I'll organise some cleanup PRs for this.

@lahm86 lahm86 merged commit 543a892 into LostArtefacts:master Aug 7, 2023
@lahm86 lahm86 deleted the issue-507-levelio-stage3 branch August 19, 2023 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants