Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Organizedlines refactor #391

Open
wants to merge 3 commits into
base: game-loader
Choose a base branch
from

Conversation

Idonotus
Copy link
Contributor

Convert organized lines to typescript and remove all gui functions from it.

// These placeholders are utilized when pieces are added or pawns promote!
const extraUndefineds = 5; // After this many promotions, need to add more undefineds and recalc the model!

class PooledArray<T> extends Array<T|undefined> {
Copy link
Contributor

@Naviary3 Naviary3 Jan 15, 2025

Choose a reason for hiding this comment

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

Overlaps with gamefileutility's TypeList type. That script has a few organized piece types, I wasn't sure where else to put them, since this script was still in javascript, but I do think it's best for our types to be defined in here.

Also, Only the pieces organized by type object has undefined placeholders for each array. I think the pieces organized by lines should get their own type.

And, since this uses a class, I want to benchmark this later to see if there's a noticeable difference in performance when loading very large games.
EDIT: Actually since PooledArrays are initialized empty, I don't think there'd be a performance change, I however may prefer a normal intersection type over a class.

}
}

interface OrganizedPieces {
Copy link
Contributor

Choose a reason for hiding this comment

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

Overlaps with gamefileutility's PiecesByType type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main branch does not have gamefileutility converted to ts. Should this be merging with gameloader?

Copy link
Contributor

@Naviary3 Naviary3 Jan 15, 2025

Choose a reason for hiding this comment

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

The main branch does not have gamefileutility converted to ts. Should this be merging with gameloader?

Oh sorry, I missed that >_< sometimes when I switch up what I'm working on I just forget on what branch I made specific changes. Yeah I'd merge it with game-loader then! It is the furthest ahead too, since I merged your move piece changes.

[line: LineKey]: Array<Piece>
}

interface Position {
Copy link
Contributor

Choose a reason for hiding this comment

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

Overlaps with gamefileutility's PiecesByKey type.

@Idonotus Idonotus changed the base branch from main to game-loader January 15, 2025 20:15
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