Add option "Unlimited" on frame limiter #324

Lyonlancer5 · 2017-10-15T06:27:25Z

Intended for use on benchmarking specific parts of the game, especially during render-intensive scenarios such as during high note densities, testing experimental sliders and the like, but can also be used to decrease input latency by indirectly affecting input polling rate.

Changes to input latency and FPS rates may vary depending on your system configuration.

itdelatrisu · 2017-10-16T04:11:29Z

I still don't really like this idea (was proposed a long time ago, #40). Personally, I find it annoying when I'm toggling FPS and suddenly hit 100% CPU usage. Towards your points, I don't believe that this game should be run past 240fps (which already seems high), and it's not the correct way to go for improving input latency.

Feel free to argue with me, I'm open to hearing more actual use cases for this.

LovelyA72 · 2017-10-17T06:31:21Z

Actually i can buy a pretty good 240hz monitor from taobao(an online shopping mall in China) for only 370 bucks. And if the max frame rate of opsu! is 240 fps, it only reach the standard frame rate of the monitor(v sync). And because osu! have this option for a long time and it is pretty stable. I recommend to add this feature.

Lemmmy · 2017-10-17T08:33:16Z

I think it would be fine to have it as a setting, but not a keyboard shortcut?

E.g. the setting can toggle between vsync, 120, 240 and unlimited, but the shortcut only vsync, 120 and 240

Lyonlancer5 · 2017-10-17T09:30:08Z

I had proposed this because the game logic update and render update are both executing on one thread (AFAIK that's how updating the game's logic and display are implemented on Slick).

Any calls to Thread.sleep(), being blocked by I/O, long/heavy computations or anything that would be CPU-intensive would delay either of the updates.

For a long-term solution though, having to separate render and logic updates into different threads would be better.

yugecin · 2017-10-17T09:52:51Z

For a long-term solution though, having to separate render and logic updates into different threads would be better.

that sounds like a pain to synchronize
you don't need separate threads, just don't render on every update https://github.com/yugecin/opsu-dance/blob/7a51828102ba94c72598d4fe66a4438f90f631aa/src/yugecin/opsudance/core/DisplayContainer.java#L246

itdelatrisu · 2017-10-18T02:33:04Z

@Lemmmy: Okay, that's a good compromise. @Lyonlancer5, if you want to update this PR to do that, I'll accept it.

@LovelyA72: I understand, but do you really notice a difference with a >240hz refresh rate compared to 120 or 240 for a 2D game?

@yugecin: Do you find that doing that actually helps? And with what kind of framerates / computer specs? I'd be surprised if the Slick2D developers would overlook something as simple as this, which is why I haven't bothered trying it.

Lyonlancer5 · 2017-10-18T10:59:42Z

Noted of @Lemmmy's idea.
I do ask, what are your PC's current specs? It barely reaches 100% CPU usage on any of my setups (except for my Pentium 4 build, that one just maxes out even at ~120FPS)

"Unlimited" would remain accessible through the Options menu

itdelatrisu · 2017-10-18T17:27:41Z

src/itdelatrisu/opsu/options/Options.java

@@ -399,15 +399,17 @@ public void selectItem(int index, GameContainer container) {

 			@Override
 			public String getValueString() {
-				return String.format((getTargetFPS() == 60) ? "%dfps (vsync)" : "%dfps", getTargetFPS());
+				return String.format((getTargetFPS() == -1) ? "Unlimited" : 


Take (getTargetFPS() == -1) ? "Unlimited" : out of String.format().

itdelatrisu · 2017-10-18T17:27:57Z

src/itdelatrisu/opsu/options/Options.java

 			}

 			@Override
 			public Object[] getItemList() {
 				if (itemList == null) {
 					itemList = new String[targetFPS.length];
 					for (int i = 0; i < targetFPS.length; i++)
-						itemList[i] = String.format((targetFPS[i] == 60) ? "%dfps (vsync)" : "%dfps", targetFPS[i]);
+						itemList[i] = String.format((targetFPS[i] == -1) ? "Unlimited" : 


(same as above)

Wait, what?

The shortcut function to "Unlimited" has already been removed. Removing this would result in the option "Unlimited" being not displayed on the option's dropdown list, but available via modifying the configuration file's FrameSync parameter to -1.

itdelatrisu · 2017-10-18T17:28:51Z

src/itdelatrisu/opsu/options/Options.java

@@ -985,7 +987,7 @@ private Options() {}
 	 * @param container the game container
 	 */
 	public static void setNextFPS(GameContainer container) {
-		int index = (targetFPSindex + 1) % targetFPS.length;
+		int index = (targetFPSindex + 1) % (targetFPS.length - 1);


Add a comment:

int index = (targetFPSindex + 1) % (targetFPS.length - 1); // Skip "Unlimited" option

yugecin · 2017-10-18T20:41:25Z

@yugecin: Do you find that doing that actually helps? And with what kind of framerates / computer specs? I'd be surprised if the Slick2D developers would overlook something as simple as this, which is why I haven't bothered trying it.

It actually feels much more responsive when I do that, and I get a bit less 100s (which makes sense because if the time between input polls is too long too much time might already been passed). It might also be a placebo effect (or something weird I did in my code), but to me it feels like it helps.

120 updates/s with 60fps already feels much smoother than 60ups & 60fps. 60fps suddenly doesn't feel like 60fps anymore.

osu! (and other rythm games) really need fast input processing, whereas many other types of games may not, so that might be why they don't give options for that. It's probably not a common thing in games to do that actually (not that I know of).

Processor: Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz (8 CPUs), ~3.4GHz
Card name: AMD Radeon (TM) R7 370 Series

Add a comment stating that changes to `index` would skip the last option by default.

Was used when calling `setNextFPS()`, displaying the notification bar when changing the target FPS rate. Not needed anymore since the shortcut call to "Unlimited" has been removed.

@tpenguinltg

Also changed FFmpeg parsing code from 0b36328 (thanks @tpenguinltg). Signed-off-by: Jeffrey Han <[email protected]>

itdelatrisu · 2018-02-12T08:23:27Z

Cleaned up and added in 6449b73.

Add option "Unlimited" FPS cap

e744292

Limit quick-toggle choices to 60, 120 and 240

a519526

"Unlimited" would remain accessible through the Options menu

itdelatrisu reviewed Oct 18, 2017

View reviewed changes

Lyonlancer5 added 2 commits October 19, 2017 06:41

Document changes to setNextFPS()

321bb82

Add a comment stating that changes to `index` would skip the last option by default.

Remove "Unlimited" value from getValueString()

197cdd1

Was used when calling `setNextFPS()`, displaying the notification bar when changing the target FPS rate. Not needed anymore since the shortcut call to "Unlimited" has been removed.

itdelatrisu added a commit that referenced this pull request Feb 12, 2018

Add "Unlimited" frame limiter option. (#324)

6449b73

Also changed FFmpeg parsing code from 0b36328 (thanks @tpenguinltg). Signed-off-by: Jeffrey Han <[email protected]>

itdelatrisu closed this Feb 12, 2018

Lyonlancer5 deleted the unlimited-fps branch February 18, 2018 01:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option "Unlimited" on frame limiter #324

Add option "Unlimited" on frame limiter #324

Lyonlancer5 commented Oct 15, 2017

itdelatrisu commented Oct 16, 2017 •

edited

Loading

LovelyA72 commented Oct 17, 2017

Lemmmy commented Oct 17, 2017 •

edited

Loading

Lyonlancer5 commented Oct 17, 2017

yugecin commented Oct 17, 2017 •

edited

Loading

itdelatrisu commented Oct 18, 2017

Lyonlancer5 commented Oct 18, 2017

itdelatrisu Oct 18, 2017

Lyonlancer5 Oct 18, 2017

itdelatrisu Oct 18, 2017

Lyonlancer5 Oct 18, 2017

itdelatrisu Oct 18, 2017

yugecin commented Oct 18, 2017 •

edited

Loading

itdelatrisu commented Feb 12, 2018

Add option "Unlimited" on frame limiter #324

Add option "Unlimited" on frame limiter #324

Conversation

Lyonlancer5 commented Oct 15, 2017

itdelatrisu commented Oct 16, 2017 • edited Loading

LovelyA72 commented Oct 17, 2017

Lemmmy commented Oct 17, 2017 • edited Loading

Lyonlancer5 commented Oct 17, 2017

yugecin commented Oct 17, 2017 • edited Loading

itdelatrisu commented Oct 18, 2017

Lyonlancer5 commented Oct 18, 2017

itdelatrisu Oct 18, 2017

Choose a reason for hiding this comment

Lyonlancer5 Oct 18, 2017

Choose a reason for hiding this comment

itdelatrisu Oct 18, 2017

Choose a reason for hiding this comment

Lyonlancer5 Oct 18, 2017

Choose a reason for hiding this comment

itdelatrisu Oct 18, 2017

Choose a reason for hiding this comment

yugecin commented Oct 18, 2017 • edited Loading

itdelatrisu commented Feb 12, 2018

itdelatrisu commented Oct 16, 2017 •

edited

Loading

Lemmmy commented Oct 17, 2017 •

edited

Loading

yugecin commented Oct 17, 2017 •

edited

Loading

yugecin commented Oct 18, 2017 •

edited

Loading