Last Played Collection - Possible to limit?

[hiulit]

@alturis If it's any relief, my script will create a backup of the gamelist.xml :P

[pjft]

@mediamogul I agree. I personally don't want to add another option to the menu to determine what's the right cutoff point, so I could pick one on my own (100, perhaps) and leave it at that.

That being said, I'm not sure I have the time to make that happen in the near future - my limited time in the coming weeks will be devoted to improving lr-fuse controls - and I genuinely doubt it's a quick change but I'll take a look if I have the time during March.

[Alturis]

Is there a github for EmulationStation?

[pjft]

@alturis https://github.com/RetroPie/EmulationStation

The change would be here:

https://github.com/RetroPie/EmulationStation/blob/master/es-app/src/CollectionSystemManager.cpp#L698

You'd also need to devise a way to remove one from the end when you add a new one to the top.

The main challenge is likely to include the top X most recently played across all systems, I imagine you'd rely on the "All Games" collection.

[hiulit]

FYI, just setting <playcount> to 0 is enough to make a game dissapear from the Last played collection. No need to remove the <lastplayed> tag.

[Alturis]

Well I found where the code change would need to go.

CollectionSystemManager.cpp

void CollectionSystemManager::populateAutoCollection(CollectionSystemData* sysData)

Just needs something like this at the end - need to read some code to figure out the right FileData collection class syntax

	if ( sysDecl.type == AUTO_LAST_PLAYED )
	{
		rootFolder->remove( max( rootFolder->count - 50, 0 ) );
	}

Edit: changed min to max

[pjft]

@alturis The idea is good, in concept, but that can only be applied if - and after - it has been sorted by last played time.

Edit: after I looked at the code I see that that particular location would already be sorted.

I'd also need to make sure to update the "update" function to keep that number straight.

I'll look through that, and I'll fix it at 100.

Edit 2: for reference, it'd be here:

https://github.com/RetroPie/EmulationStation/blob/master/es-app/src/CollectionSystemManager.cpp#L280

[hiulit]

@pjft Nice! Does this mean that it will be added to EmulationStarion at some point? Also, could the number be edited by the user?
Anyway, it's been fun playing with my script :P I'll release it anyway when I finish it, at least for learning purposes.

[pjft]

@hiulit "At some point", yes.

Edited by user, no. As I said, I don't want to perpetuate a growing trend of adding yet another option to ES.

If anyone strongly objects to 100 and/or feels as a wide community that there is a consensual number that makes more sense, I can consider making that the default. But if everyone has different opinions, I'll quote Jim Barksdale:

"If we have data, let’s look at data. If all we have are opinions, let’s go with mine."

[hiulit]

@pjft I wasn't trying to put any pressure, at all, btw ;)
And about the number, I think it might be a little too high. Maybe 25? But that's just a personal choice, of course. I'm fine with whatever number you decide :)

[mediamogul]

@pjft said in Last Played Collection - Possible to limit?:

If anyone strongly objects to 100 and/or feels as a wide community that there is a consensual number that makes more sense

Personally, I think 100 has a nice, rounded feel to it. At first blush, 50 seems like more than enough, but if there is to be no setting, and I fully understand the aversion to adding one, 100 covers more ground and should please the most people.

[mediamogul]

@pjft

Out of curiosity, is it bad practice to have a back-end only setting. Like say having the cutoff be 100 by default, but adjustable from es_settings.cfg?

[Alturis]

@mediamogul said in Last Played Collection - Possible to limit?:

Out of curiosity, is it bad practice to have a back-end only setting. Like say having the cutoff be 100 by default, but adjustable from es_settings.cfg?

That was my thought as well. Make it 100 unless someone cares enough to edit it in the cfg

I already feel bad for bringing it up and adding work to @pjft's plate. ;)

[mediamogul]

@alturis

I don't think it ever hurts to discuss these things. Ideally, after talking through it, we should really open up a ticket on the github referencing this thread. From there, it'll be considered alongside everything else based on merit and the interest to see it resolved.

[pjft]

Not at all, and apologies if my answers seem a bit more rushed - time hasn't been abundant here, especially during the day.

I was looking into this right now, but I see that I fell into the old trap of believing that this had an easy solution, and that @Alturis 's suggestion would indeed work out of the box :)

Alas that's not to be, that would work partially if rootFolder was indeed a vector or something similar, but it's not. It's alright, we'll figure something out. I'll run with 50, as there seemed to be a reasonable preference for that number and it's still a few page scrolls.

In regards to the back-end only setting, I don't think there's any specific guidance or preference in that regard. There are a few guiding principles I try - or tend - to adhere to, but I'm not dogmatic about them.

My vision/perspective for ES is that it should be accessible and should not require someone to jump through hoops to make things run as intended. So that's check #1.

Second. options that rely on users to edit something manually can be error prone and result in situations that may break beyond your control. Think of a simple thing such as the frequent complaints that a new system isn't showing up because someone has a customized es_systems.cfg file. In this case, what if someone decides to make it a negative number? Zero? A number that's larger than the actual accepted integer range? A letter?

I know, one can claim that that possibility is already there because the options are a text file, but since we aren't in any way encouraging that use case, if someone mucks it up it's really purely their responsibility. From the moment we open that to others, it just becomes... meh.

The final check is that, if it's an option that will indeed be used by many, then it should be a visible option, otherwise it just reflects some sort of lack of decisiveness/thought process. "The Power of Defaults" is a good reading on that, and sometimes I feel that an excessive number of options stems from poor judgement or laziness - I have certainly been a key culprit of adding a lot of options when I was doing both the OMX Player work as well as the Collections one that these days, in hindsight, I probably wouldn't have added. The fact that some people may use them doesn't mean that they would be worse off with a different one by default, but the overall user experience in the end suffers as well as the code readability. Think of all the ES options that you probably never touched or left at their default settings. The comfort of knowing that things could be changed likely doesn't offset the pain that someone has in creating them, maintaining them, and keeping the menus from becoming a giant mess. :)

This case in particular is a fairly small thing - pick a number, any number. We'll start with 50, as it is the number most of you seem to gravitate towards, and then we'll run it for a while. If we feel it's too small, we'll change it.

Also, I doubt Buzz and the other folks would be happy with such a back-end option :)

[mediamogul]

@pjft

All very sober considerations.

[pjft]

Here you go:

https://github.com/RetroPie/EmulationStation/pull/383

If anyone does want to test it out for bugs and such, I'd certainly appreciate a second pair of hands/eyes on it.

Best.

[darknior]

Thanks a lot, really good idea :)

[Alturis]

@pjft wouldnt you just expose a new public method of FileData that manipulates the mChildren table to remove X elements from the end?

Simiar to the sort function itself?

void FileData::sort(ComparisonFunction& comparator, bool ascending)
{
	std::stable_sort(mChildren.begin(), mChildren.end(), comparator);

	for(auto it = mChildren.cbegin(); it != mChildren.cend(); it++)
	{
		if((*it)->getChildren().size() > 0)
			(*it)->sort(comparator, ascending);
	}

	if(!ascending)
		std::reverse(mChildren.begin(), mChildren.end());
}

void FileData::remove(  int rmvCount )

[Alturis]

@alturis said in Last Played Collection - Possible to limit?:

@pjft wouldnt you just expose a new public method of FileData that manipulates the mChildren table to remove X elements from the end?

Simiar to the sort function itself?

Nevermind. Looks like you already took care of this. Missed that. Thank you!