Filter empty sitreps
Moderator: Committer
Filter empty sitreps
When browsing sitreps pages, it annoyed me when displaying empty pages when turns didn't get anything to report.
The attached patch fixes it for me.
The attached patch fixes it for me.
All the patches I'll provide for freeorion will be released under the GPL v2 or later license.
Let's unleash the dyson forest powa!
Let's unleash the dyson forest powa!
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13603
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Filter empty sitreps
Not a fan of the do / while loops there... Haven't looked closely, but any use of that sort makes me worried about unexpected cases that won't actually exit...
Re: Filter empty sitreps
I've chosen that form because it makes the code smaller, looks cleaner and natural to me, but that's a matter of taste, so...
Do you want me to redo it with a plain while loop ? There will be duplicated code...
Do you want me to redo it with a plain while loop ? There will be duplicated code...
All the patches I'll provide for freeorion will be released under the GPL v2 or later license.
Let's unleash the dyson forest powa!
Let's unleash the dyson forest powa!
Re: Filter empty sitreps
Seems to me the most reliable way to ensure against an infinite loop would be to add a paging concept -- goes to next/prev nonempty sitrep or pages 20 turns if no non-empties in the interim. if you used an int initial_turn = m_showing_turn rather than the prev_turn = 0 then I think it would be no longer than your current patch.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0
Re: Filter empty sitreps
Why do you think an infinite loop may happen ?Dilvish wrote:Seems to me the most reliable way to ensure against an infinite loop
I don't think I understand that. Do you mean that if there are 30 empty sitrep turns, on the first back it'll go back 20, and on the next back it'll go to the first non empty, back 10 turns more ? What would that buy us ? Either we don't want to display empty sitreps at all or we display them all. Displaying only some of them looks rather pointless. If it's something else, please elaborate.Dilvish wrote:a paging concept -- goes to next/prev nonempty sitrep or pages 20 turns if no non-empties in the interim
All the patches I'll provide for freeorion will be released under the GPL v2 or later license.
Let's unleash the dyson forest powa!
Let's unleash the dyson forest powa!
Re: Filter empty sitreps
Hmm, I guess because I hadn't looked closely enoughvincele wrote:Why do you think an infinite loop may happen ?
'Pointless' seems a bit on the strong side But yes, you understood the idea, and I can accept the fact that it is Not What You Had In Mind.I don't think I understand that. Do you mean that if there are 30 empty sitrep turns, on the first back it'll go back 20, and on the next back it'll go to the first non empty, back 10 turns more ? What would that buy us ? Either we don't want to display empty sitreps at all or we display them all. Displaying only some of them looks rather pointless. If it's something else, please elaborate.Dilvish wrote:a paging concept -- goes to next/prev nonempty sitrep or pages 20 turns if no non-empties in the interim
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0
Re: Filter empty sitreps
OK, I feared I missed something. It works for me, I think I now don't even notice it since when going back through sitreps nothing looks strange and I don't even think about it any more.Dilvish wrote:Hmm, I guess because I hadn't looked closely enoughvincele wrote:Why do you think an infinite loop may happen ?
Hey that was why rather was added, to tone it down a bit We can have an option to make it display empty sitreps as before, or display them in new creative ways, I just coded what I wanted for myself, but am prepared to add more to the patch to please others... The patch in its present form is a better base for discussions than nothing...'Pointless' seems a bit on the strong side But yes, you understood the idea, and I can accept the fact that it is Not What You Had In Mind.
So people, tell us what you want and we'll see what is sensible.
All the patches I'll provide for freeorion will be released under the GPL v2 or later license.
Let's unleash the dyson forest powa!
Let's unleash the dyson forest powa!
Re: Filter empty sitreps
I'm happy to just skip multiple turns, but it might be helpful if it says you're skipping multiple turns somehow. And yeah, an option to turn it back on, there's bound to be an edge case somewhere down the line...
Mat Bowles
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.
Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.
Re: Filter empty sitreps
A sitrep "N turns of nothing of importance"?MatGB wrote:I'm happy to just skip multiple turns, but it might be helpful if it says you're skipping multiple turns somehow. And yeah, an option to turn it back on, there's bound to be an edge case somewhere down the line...
https://github.com/mmoderau
[...] for Man has earned his right to hold this planet against all comers, by virtue of occasionally producing someone totally batshit insane. - Randall Munroe, title text to xkcd #556
[...] for Man has earned his right to hold this planet against all comers, by virtue of occasionally producing someone totally batshit insane. - Randall Munroe, title text to xkcd #556
Re: Filter empty sitreps
Although I wasn't so hot on this to begin with, as time has passed, I've encountered more situations where I wish we had this feature in place. Geoff, after the bit of discussion above are you content with the patch, or would you prefer to see it structured with a loop?
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13603
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Filter empty sitreps
Did you mean "without"?Dilvish wrote:...are you content with the patch, or would you prefer to see it structured with a loop?
I'd prefer without the current loops, which I think involve an N^2 searching scheme (with repeated calls to GetTurnSitrepsFromEmpire while incrementing the turn searched by 1 each time). Instead, I'd prefer to search through all the available sitreps once, to find then next or previous turn on which any are available, then set that as the new m_showing_turn.
Re: Filter empty sitreps
Here is an updated version, hopefully without the quadratic complexity.
In addition to that it has also the ability to go back to the initial sitrep (before turn 1), that I think should stay reachable, because it contains valuable information.
In addition to that it has also the ability to go back to the initial sitrep (before turn 1), that I think should stay reachable, because it contains valuable information.
All the patches I'll provide for freeorion will be released under the GPL v2 or later license.
Let's unleash the dyson forest powa!
Let's unleash the dyson forest powa!
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13603
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Filter empty sitreps
I think it'd be better to loop over the sitreps once, keeping track of the next higher / lower turn number on which a suitable sitrep appears, then set that as the new value of m_showing_turn.
Re: Filter empty sitreps
I don't think I understand what you want, but let me try...
You want to only iterate through the sitreps once *per turn* and keep that data between calls to PrevClicked() and NextClicked(), but this will need to be invalidated somehow at the next turn.
You want to only iterate through the sitreps once *per turn* and keep that data between calls to PrevClicked() and NextClicked(), but this will need to be invalidated somehow at the next turn.
All the patches I'll provide for freeorion will be released under the GPL v2 or later license.
Let's unleash the dyson forest powa!
Let's unleash the dyson forest powa!
- Geoff the Medio
- Programming, Design, Admin
- Posts: 13603
- Joined: Wed Oct 08, 2003 1:33 am
- Location: Munich
Re: Filter empty sitreps
No, just iterate through once when the next / prev buttons are clicked. Currently you iterate through all the sitreps, storing them in a map (order NlogN, for N sitreps I think?), then do a linear loop over turns (probably effectively constant time in practice, but possibly order T for T turns) and for each turn do an search to see if there are any sitreps for that turn (order logN).vincele wrote:You want to only iterate through the sitreps once *per turn* and keep that data...
Instead, I'm suggesting just iterating over all the sitreps, and keep track of the next bigger / smaller turn at which one appears relative to the current turn. This would be just order N for N sitreps. If the sitreps were already sorted, you could make this search order logN, but I don't think they are or that it's worth sorting them or caching the list of turn numbers for which sitreps are available just for this purpose.
My proposal also avoids the odd-to-me looking do / while loops.