Skip to content

Commit

Permalink
Fixes Knobs not working, Sliders jamming, and Sliders floating-point …
Browse files Browse the repository at this point in the history
…rounding error (ParadiseSS13#23299)

* Fix slider misconfiguration

* Parse input numbers before passing on

* Convert to origin-based code

* Apply dragmatrix to other offset components

* Remove size-based code

* Use state to track original value instead of using props

* Build and update tgui

* Remove reliance of offset on window position

* Fix Knob usage

* Build and update tgui

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

* ci rerun

---------

Co-authored-by: Arthri <[email protected]>
Co-authored-by: tgui <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 12, 2023
1 parent 72477a9 commit 174ea96
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 87 deletions.
85 changes: 31 additions & 54 deletions tgui/packages/tgui/components/DraggableControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ export class DraggableControl extends Component {
super(props);
this.inputRef = createRef();
this.state = {
originalValue: props.value,
value: props.value,
dragging: false,
editing: false,
oldOffset: null,
origin: null,
suppressingFlicker: false,
};

Expand Down Expand Up @@ -50,8 +51,10 @@ export class DraggableControl extends Component {
document.body.style['pointer-events'] = 'none';
this.ref = e.currentTarget;
this.setState({
originalValue: value,
dragging: false,
value,
origin: getScalarScreenOffset(e, dragMatrix),
});
this.timer = setTimeout(() => {
this.setState({
Expand Down Expand Up @@ -82,57 +85,16 @@ export class DraggableControl extends Component {
}
this.setState((prevState) => {
const state = { ...prevState };
const oldOffset = prevState.oldOffset;
const offset =
getScalarScreenOffset(e, dragMatrix) -
this.ref.getBoundingClientRect().left -
window.screenX;
const origin = prevState.origin;
const offset = getScalarScreenOffset(e, dragMatrix) - origin;
if (prevState.dragging) {
if (
oldOffset !== undefined &&
oldOffset !== null &&
offset !== oldOffset
) {
const maxStep = maxValue / step;
const toNearestStep =
offset > oldOffset
? Math.floor // Increasing
: Math.ceil; // Decreasing
/* ● = step, o = oldOffset, n = offset
* There are four cases to consider for the following code:
* Case 1: Increasing(offset > oldOffset), moving between steps
* ●--o--n-●
* value should not change. Since both offsets are subject to floor,
* they have the same nearest steps and the difference cancels out,
* leaving value the same
* Case 2: Decreasing(offset < oldOffset), moving between steps
* ●--n--o-●
* Same as Case 1 except the function is ceil not floor
* Case 3: Increasing, offset is past step
* ●-o-●-n-● ; ●-o-●---●-n
* value should increase by 1, or however many steps o is behind n
* Case 4: Decreasing, offset is behind step
* ●-n-●-o-● ; ●-n-●---●-o
* Same as Case 3, but decrease instead of increase
*/
const oldStep = clamp(
toNearestStep(oldOffset / stepPixelSize),
0,
maxStep
);
const newStep = clamp(
toNearestStep(offset / stepPixelSize),
0,
maxStep
);
const stepDifference = newStep - oldStep;
state.value = clamp(
state.value + stepDifference * step,
minValue,
maxValue
);
}
state.oldOffset = offset;
const stepDifference = Math.trunc(offset / stepPixelSize);
state.value = clamp(
Math.floor(state.originalValue / step) * step +
stepDifference * step,
minValue,
maxValue
);
} else if (Math.abs(offset) > 4) {
state.dragging = true;
}
Expand All @@ -147,9 +109,10 @@ export class DraggableControl extends Component {
clearTimeout(this.timer);
clearInterval(this.dragInterval);
this.setState({
originalValue: null,
dragging: false,
editing: !dragging,
oldOffset: null,
origin: null,
});
document.removeEventListener('mousemove', this.handleDragMove);
document.removeEventListener('mouseup', this.handleDragEnd);
Expand Down Expand Up @@ -228,7 +191,14 @@ export class DraggableControl extends Component {
if (!editing) {
return;
}
const value = clamp(e.target.value, minValue, maxValue);
const inputValue = parseInt(e.target.value, 10);
if (isNaN(inputValue) || e.target.value.match(/[^0-9]/g)) {
this.setState({
editing: false,
});
return;
}
const value = clamp(inputValue, minValue, maxValue);
this.setState({
editing: false,
value,
Expand All @@ -243,7 +213,14 @@ export class DraggableControl extends Component {
}}
onKeyDown={(e) => {
if (e.keyCode === 13) {
const value = clamp(e.target.value, minValue, maxValue);
const inputValue = parseInt(e.target.value, 10);
if (isNaN(inputValue) || e.target.value.match(/[^0-9]/g)) {
this.setState({
editing: false,
});
return;
}
const value = clamp(inputValue, minValue, maxValue);
this.setState({
editing: false,
value,
Expand Down
6 changes: 3 additions & 3 deletions tgui/packages/tgui/components/NanoMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ const NanoMapZoomer = (props, context) => {
<LabeledList>
<LabeledList.Item label="Zoom">
<Slider
minValue="1"
maxValue="8"
stepPixelSize="10"
minValue={1}
maxValue={8}
stepPixelSize={10}
format={(v) => v + 'x'}
value={props.zoom}
onDrag={(e, v) => props.onZoom(e, v)}
Expand Down
16 changes: 8 additions & 8 deletions tgui/packages/tgui/interfaces/DNAModifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ const DNAModifierMainUI = (props, context) => {
<LabeledList>
<LabeledList.Item label="Target">
<Knob
minValue="1"
maxValue="15"
minValue={1}
maxValue={15}
stepPixelSize="20"
value={selectedUITarget}
format={(value) => value.toString(16).toUpperCase()}
Expand Down Expand Up @@ -272,9 +272,9 @@ const DNAModifierMainRadiationEmitter = (props, context) => {
<LabeledList>
<LabeledList.Item label="Intensity">
<Knob
minValue="1"
maxValue="10"
stepPixelSize="20"
minValue={1}
maxValue={10}
stepPixelSize={20}
value={radiationIntensity}
popUpPosition="right"
ml="0"
Expand All @@ -283,9 +283,9 @@ const DNAModifierMainRadiationEmitter = (props, context) => {
</LabeledList.Item>
<LabeledList.Item label="Duration">
<Knob
minValue="1"
maxValue="20"
stepPixelSize="10"
minValue={1}
maxValue={20}
stepPixelSize={10}
unit="s"
value={radiationDuration}
popUpPosition="right"
Expand Down
28 changes: 14 additions & 14 deletions tgui/packages/tgui/interfaces/Instrument.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,10 @@ const InstrumentStatus = (properties, context) => {
<LabeledList.Item label="Repeat">
<Slider
animated
minValue="0"
minValue={0}
maxValue={maxRepeats}
value={repeat}
stepPixelSize="59"
stepPixelSize={59}
onChange={(_e, v) =>
act('repeat', {
new: v,
Expand Down Expand Up @@ -404,7 +404,7 @@ const InstrumentStatus = (properties, context) => {
minValue={minVolume}
maxValue={maxVolume}
value={volume}
stepPixelSize="6"
stepPixelSize={6}
onDrag={(_e, v) =>
act('setvolume', {
new: v,
Expand Down Expand Up @@ -447,11 +447,11 @@ const InstrumentStatusAdvanced = (properties, context) => {
smt = 'Linear';
modebody = (
<Slider
minValue="0.1"
maxValue="5"
minValue={0.1}
maxValue={5}
value={sustainLinearDuration}
step="0.5"
stepPixelSize="85"
step={0.5}
stepPixelSize={85}
format={(v) => round(v * 100) / 100 + ' seconds'}
onChange={(_e, v) =>
act('setlinearfalloff', {
Expand All @@ -464,10 +464,10 @@ const InstrumentStatusAdvanced = (properties, context) => {
smt = 'Exponential';
modebody = (
<Slider
minValue="1.025"
maxValue="10"
minValue={1.025}
maxValue={10}
value={sustainExponentialDropoff}
step="0.01"
step={0.01}
format={(v) => round(v * 1000) / 1000 + '% per decisecond'}
onChange={(_e, v) =>
act('setexpfalloff', {
Expand Down Expand Up @@ -509,7 +509,7 @@ const InstrumentStatusAdvanced = (properties, context) => {
minValue={noteShiftMin}
maxValue={noteShiftMax}
value={noteShift}
stepPixelSize="2"
stepPixelSize={2}
format={(v) =>
v + ' keys / ' + round((v / 12) * 100) / 100 + ' octaves'
}
Expand All @@ -535,10 +535,10 @@ const InstrumentStatusAdvanced = (properties, context) => {
<LabeledList.Item label="Volume Dropoff Threshold">
<Slider
animated
minValue="0.01"
maxValue="100"
minValue={0.01}
maxValue={100}
value={sustainDropoffVolume}
stepPixelSize="6"
stepPixelSize={6}
onChange={(_e, v) =>
act('setdropoffvolume', {
new: v,
Expand Down
12 changes: 6 additions & 6 deletions tgui/packages/tgui/interfaces/OperatingComputer.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,10 @@ const OperatingComputerOptions = (props, context) => {
<LabeledList.Item label="Health Announcer Threshold">
<Knob
bipolar
minValue="-100"
maxValue="100"
minValue={-100}
maxValue={100}
value={healthAlarm}
stepPixelSize="5"
stepPixelSize={5}
ml="0"
onChange={(e, val) =>
act('health_adj', {
Expand All @@ -226,10 +226,10 @@ const OperatingComputerOptions = (props, context) => {
<LabeledList.Item label="Oxygen Alarm Threshold">
<Knob
bipolar
minValue="-100"
maxValue="100"
minValue={-100}
maxValue={100}
value={oxyAlarm}
stepPixelSize="5"
stepPixelSize={5}
ml="0"
onChange={(e, val) =>
act('oxy_adj', {
Expand Down
4 changes: 2 additions & 2 deletions tgui/packages/tgui/public/tgui.bundle.js

Large diffs are not rendered by default.

0 comments on commit 174ea96

Please sign in to comment.