patternjavaMinor
Event-based Xbox Controller polling
Viewed 0 times
pollingcontrollerbasedxboxevent
Problem
The goal of the following code is to be able to use an Xbox Controller in a Java program, it works with the jinput library.
Unfortunately this library is rather old, has little to no documentation and uses a polling system. There are some existing event-based (xbox) controller input systems, but most of them are embedded in 3D game engines, so I got the idea to make my own library, these are the ultimate goals:
These goals have not been met yet, as currently it just wraps over the jinput library.
In order to understand some of the weird stuff that is going on in my classes, you will need to understand the following things that are going on in the jinput library:
Some relevant information about jinput can be found in this topic rather than in non-existing documentation.
With that said, I have implemented the following:
Unfortunately this library is rather old, has little to no documentation and uses a polling system. There are some existing event-based (xbox) controller input systems, but most of them are embedded in 3D game engines, so I got the idea to make my own library, these are the ultimate goals:
- It should support multiple types of controllers.
- It should have some kind of plug-in system for the native DLLs, so instead of jinput I should be able to use some other library.
These goals have not been met yet, as currently it just wraps over the jinput library.
In order to understand some of the weird stuff that is going on in my classes, you will need to understand the following things that are going on in the jinput library:
- The left and right triggers are represented as a single z-axis, so you are not able to identify if the left or right trigger is held down, you instead get an average value of both triggers. If you hold down the left trigger only, you get a value of 1, if you hold down the right trigger only you get a value of -1 and if you hold down both triggers or neither, then you get a value of 0.
- It uses 0f and 1f to represent not pressed and pressed button states.
- For the D-pad (directional pad) it uses the values 0f, 0.125f, 0.25f, 0.375f, 0.5f, 0.625f, 0.75f, 0.875f and 1f for respectively the off state and the possible buttons that have been pressed down.
- The event system provided by jinput is non-blocking, meaning that still need to loop every time interval and read all events that have occured since last time.
- I haven't been able to get the rumblers of the controller to work, so there is no support for this at all in my library.
Some relevant information about jinput can be found in this topic rather than in non-existing documentation.
With that said, I have implemented the following:
- An event-based system wher
Solution
A few thoughts, based on general concerns
In particular
I don't know what the right answer is for the controller library; but the library should be written in such a way that the answers are obvious to the users.
It seems to me that the XboxGamepad you have implemented has too many different responsibilities.
For instance, I made an attempt at refactoring the
So the first thing I see is that XboxGamepad doesn't need a
Then startListening would look like
Which didn't buy me as much as I had hoped. What I think the code should look like is
if you can re-use the same task after the next stop/start, or
if you need a new instance every time around. The code looks cleaner, but it doesn't eliminate the need to create an inner class instance somewhere.
But if can separate the polling from the LifeCycle management, then we get a much nicer line. From your example, I'm imagining something like
Where Builder.build() looks something like....
Again, if the pollTask can be different after each start, then a pollTaskFactory would be passed in instead.
If I were implementing this library, I'd want to go the extra mile to use a Fluent Builder. And I would also consider
If all of your events are going to be handling time the same way, then you should probably be implementing an
The Deadzone code looks very suspicious
```
private static class Deadzone {
private Component eventAxis;
private final List axes;
private final DeadzoneType deadzoneType;
private final float value;
private Deadzone(Component eventAxis, List
startListening and stopListening suggest lifecycle management, which in turn suggests that there is a state machine here. The fact that you are throwing IllegalStateException is supporting evidence. But your state machine implementation is currently implicit. You should make it explicit.In particular
- Can
startListeningbe called again afterstopListeninghas been invoked?
- Does it make sense to call
addListenerorsetDeadzoneafterstartListeninghas been invoked?
I don't know what the right answer is for the controller library; but the library should be written in such a way that the answers are obvious to the users.
It seems to me that the XboxGamepad you have implemented has too many different responsibilities.
- In your example, where you are invoking
addListenerandsetDeadzone, it looks like aBuilder
- In
startListeningandstopListening, it looks like aLifeCycleManager
- In
startListening, it appears to be aFactory
- In
poll, it's anEventListener
For instance, I made an attempt at refactoring the
startListening method.public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = new Timer(true);
this.pollTimer.scheduleAtFixedRate(new TimerTask() {
@Override
public void run() {
poll();
}
}, 0, pollDelay);
}So the first thing I see is that XboxGamepad doesn't need a
pollDelay itself; it is just passing it through to somebody else. For instance, suppose we had a little PollingScheduleFactory, implemented so:class PollingScheduleFactory {
private final int pollDelay;
public Timer schedule(final Runnable task) {
Timer pollTimer = new Timer(true);
pollTimer.scheduleAtFixedRate(new TimerTask() {
@Override
public void run() {
task.run();
}
}, 0, pollDelay);
return pollTimer;
}
}Then startListening would look like
public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = this.pollingScheduleFactory.schedule(new Runnable() {
@Override
public void run() {
poll();
}
});
}Which didn't buy me as much as I had hoped. What I think the code should look like is
public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = this.pollingScheduleFactory.schedule(pollTask);
}if you can re-use the same task after the next stop/start, or
public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = this.pollingScheduleFactory.schedule(pollTaskFactory.newTask());
}if you need a new instance every time around. The code looks cleaner, but it doesn't eliminate the need to create an inner class instance somewhere.
But if can separate the polling from the LifeCycle management, then we get a much nicer line. From your example, I'm imagining something like
XboxGamepad.Builder builder = new XboxGamepad.Builder(controller, 10);
builder.setDeadzone(...);
builder.addListener(...);
XboxGamepad gamepad = builder.build();
gamepad.startListening();Where Builder.build() looks something like....
public XboxGamepad build () {
PollTask pollTask = new PollTask(gamepad, eventListeners, ...);
PollingScheduleFactory scheduleFactory = new PollScheduledFactory(pollDelay);
...
return new XboxGamepad(pollTask, scheduleFactory);
}Again, if the pollTask can be different after each start, then a pollTaskFactory would be passed in instead.
If I were implementing this library, I'd want to go the extra mile to use a Fluent Builder. And I would also consider
setLinearDeadzone and setRadialDeadzone -- a Fluent Builder, or a family of Fluent Builders, can take a lot of the burden off of your clients without necessarily removing the flexibility offered by the generic interface.public interface Event {
long getTime();
}If all of your events are going to be handling time the same way, then you should probably be implementing an
AbstractEvent that manages that for you. abstract class AbstractEvent implements Event {
private final long time;
protected AbstractEvent(long time) {
this.time = time;
}
long getTime() {
return this.time;
}
}The Deadzone code looks very suspicious
```
private static class Deadzone {
private Component eventAxis;
private final List axes;
private final DeadzoneType deadzoneType;
private final float value;
private Deadzone(Component eventAxis, List
Code Snippets
public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = new Timer(true);
this.pollTimer.scheduleAtFixedRate(new TimerTask() {
@Override
public void run() {
poll();
}
}, 0, pollDelay);
}class PollingScheduleFactory {
private final int pollDelay;
public Timer schedule(final Runnable task) {
Timer pollTimer = new Timer(true);
pollTimer.scheduleAtFixedRate(new TimerTask() {
@Override
public void run() {
task.run();
}
}, 0, pollDelay);
return pollTimer;
}
}public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = this.pollingScheduleFactory.schedule(new Runnable() {
@Override
public void run() {
poll();
}
});
}public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = this.pollingScheduleFactory.schedule(pollTask);
}public void startListening() {
if (pollTimer != null) {
throw new IllegalStateException("You are already listening to events");
}
pollTimer = this.pollingScheduleFactory.schedule(pollTaskFactory.newTask());
}Context
StackExchange Code Review Q#107143, answer score: 4
Revisions (0)
No revisions yet.