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

LBP3 Playlists #702

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

Toastbrot236
Copy link
Contributor

@Toastbrot236 Toastbrot236 commented Dec 15, 2024

Implements the ability to create, view, manage etc. playlists in LBP3. They get saved as GamePlaylists in the database, which among some other things allows lbp3 playlists to show up in lbp1 as lbp1 playlists and vice versa. This PR also adds the ability to heart playlists in LBP3 with FavouritePlaylistRelations, aswell as an index number in LevelPlaylistRelations to also support setting custom level orders inside playlists in LBP3. Also implements actually assigning creation and last updated dates to playlists when creating them in general.

Edit: This now also lets the endpoint for adding a slot to a playlist in lbp1 catch and block more ways of creating recursive playlists, and it also prevents TraverseParentsRecursively in GamePlaylistExtensions from causing an overflow once it gets stuck in a loop in the tree it's supposed to traverse.

Copy link
Member

@Beyley Beyley left a comment

Choose a reason for hiding this comment

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

just an initial run-through, looks quite good aside from minor stuff

Comment on lines 144 to 145
if (!int.TryParse(levelIdStr, out int levelId)) return null;
if (!int.TryParse(levelIdStr.StartsWith('d') ? levelIdStr[1..] : levelIdStr, out int levelId)) continue;
GameLevel? level = database.GetLevelById(levelId);
Copy link
Member

Choose a reason for hiding this comment

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

Could you document here what this extra d check is for?

Comment on lines 121 to 122
// Setting TokenGame to LBP3 to get the true amount of levels in the playlist
Index = this.GetTotalLevelsInPlaylistCount(parent, TokenGame.LittleBigPlanet3),
Copy link
Member

Choose a reason for hiding this comment

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

Probably worth adding a game-agnostic function to get this count, in-case we ever want to expand the playlist system to LBP Vita
I have ideas of using slot overrides to achieve this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

game-agnostic as in not game-dependent, one that doesnt take a TokenGame?

Comment on lines 188 to 190
// Only sort by order if needed, to improve performance
if (game == TokenGame.LittleBigPlanet3)
relations = relations.OrderBy(r => r.Index);
Copy link
Member

Choose a reason for hiding this comment

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

Given theres ideas of using slot overrides to expand this system to other games than LBP1/3, its worth removing this, the amount of time spent ordering is going to be very small (especially after we have Postgres)

Suggested change
// Only sort by order if needed, to improve performance
if (game == TokenGame.LittleBigPlanet3)
relations = relations.OrderBy(r => r.Index);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you are suggesting to remove custom level ordering as a whole and to just not support that feature, because of potential playlist support in other games using level overrides? I am not understanding the link between the two.

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