Skip to content

Commit 71635ba

Browse files
ImperaZimImperaZim
committed
Extract OptionNormalizerTrait, fix index clamping, remove LongForm cachedButtons
- Extract OptionNormalizerTrait shared by Dropdown, StepSlider, SearchableDropdown - Fix StepSlider/SearchableDropdown index clamping: max(0, min($default, $count-1)) - LongForm: remove cachedButtons property, use local variable in serializeFormData - LongForm: recalculate buttons in handleResponse instead of using stale cache - FormChain: fix closure return type hints - CustomForm/DynamicCustomForm: minor consistency fixes - ValidatedInput: improve validation error messages - All form elements: catch Throwable consistently
1 parent 36abcdb commit 71635ba

23 files changed

Lines changed: 249 additions & 272 deletions

‎src/imperazim/form/FormData.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,16 @@ public function get(string|int $key): mixed {
9898
return $this->offsetGet($key);
9999
}
100100

101+
/**
102+
* Checks if a key exists (even if null).
103+
*
104+
* @param string|int $key Data key
105+
* @return bool True if key exists
106+
*/
107+
public function has(string|int $key): bool {
108+
return array_key_exists($key, $this->data);
109+
}
110+
101111
/**
102112
* Converts data to array.
103113
*

‎src/imperazim/form/LibForm.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types = 1);
4+
35
namespace imperazim\form;
46

57
use pocketmine\plugin\PluginBase;

‎src/imperazim/form/chain/FormChain.php‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
*/
2727
final class FormChain {
2828

29+
/** @var \WeakReference<Player> Target player */
30+
private \WeakReference $playerRef;
31+
2932
/** @var list<array{factory: Closure, skip: Closure|null}> */
3033
private array $steps = [];
3134

@@ -35,9 +38,10 @@ final class FormChain {
3538
private int $currentStep = 0;
3639

3740
private function __construct(
38-
private Player $player,
41+
Player $player,
3942
array|FormData $initialData = []
4043
) {
44+
$this->playerRef = \WeakReference::create($player);
4145
$this->data = $initialData instanceof FormData ? $initialData : new FormData($initialData);
4246
}
4347

@@ -109,8 +113,9 @@ public function advance(array $stepData = []): void {
109113
* Cancels the chain at the current step.
110114
*/
111115
public function cancel(): void {
112-
if ($this->onCancelCallback !== null) {
113-
($this->onCancelCallback)($this->player, $this->data, $this->currentStep);
116+
$player = $this->playerRef->get();
117+
if ($this->onCancelCallback !== null && $player !== null) {
118+
($this->onCancelCallback)($player, $this->data, $this->currentStep);
114119
}
115120
}
116121

@@ -148,20 +153,25 @@ private function advanceTo(int $index): void {
148153

149154
$this->currentStep = $index;
150155

156+
$player = $this->playerRef->get();
157+
if ($player === null) {
158+
return;
159+
}
160+
151161
// All steps completed
152162
if ($index >= count($this->steps)) {
153163
if ($this->onCompleteCallback !== null) {
154-
($this->onCompleteCallback)($this->player, $this->data);
164+
($this->onCompleteCallback)($player, $this->data);
155165
}
156166
return;
157167
}
158168

159169
// Create and send next form
160170
$step = $this->steps[$index];
161-
$form = ($step['factory'])($this->player, $this->data);
171+
$form = ($step['factory'])($player, $this->data);
162172

163173
if ($form instanceof Form) {
164-
$form->sendTo($this->player);
174+
$form->sendTo($player);
165175
}
166176
}
167177
}

‎src/imperazim/form/custom/CustomForm.php‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
namespace imperazim\form\custom;
66

77
use JsonSerializable;
8-
use InvalidArgumentException;
98
use pocketmine\player\Player;
9+
use pocketmine\form\FormValidationException;
1010
use imperazim\form\Form;
1111
use imperazim\form\FormData;
1212
use imperazim\form\FormResult;
1313
use imperazim\form\base\elements\Title;
1414
use imperazim\form\custom\elements\ElementCollection;
15+
use imperazim\form\custom\elements\ValidatedInput;
1516
use imperazim\form\custom\response\CustomResponse;
1617

1718
/**
@@ -24,8 +25,8 @@ abstract class CustomForm extends Form {
2425
/** @var FormData Form data container */
2526
private FormData $formData;
2627

27-
/** @var Player Target player */
28-
private Player $player;
28+
/** @var \WeakReference<Player> Target player */
29+
private \WeakReference $playerRef;
2930

3031
/** @var bool Bypass validation */
3132
private bool $force;
@@ -42,7 +43,7 @@ public function __construct(
4243
array|FormData $data = [],
4344
bool $force = false
4445
) {
45-
$this->player = $player;
46+
$this->playerRef = \WeakReference::create($player);
4647
$this->force = $force;
4748
$this->formData = $data instanceof FormData ? $data : new FormData($data);
4849

@@ -101,9 +102,9 @@ abstract protected function onClose(
101102
* @param Player $player Responding player
102103
* @param mixed $raw Raw response data
103104
*/
104-
public function handleResponse(Player $player, $raw): void {
105+
public function handleResponse(Player $player, mixed $raw): void {
105106
if ($raw !== null && !is_array($raw)) {
106-
throw new InvalidArgumentException("Response data must be array or null.");
107+
throw new FormValidationException("Response data must be array or null.");
107108
}
108109

109110
$elements = $this->getCachedElements($player, $this->formData);
@@ -112,10 +113,17 @@ public function handleResponse(Player $player, $raw): void {
112113

113114
if (is_array($raw)) {
114115
foreach ($elements as $idx => $element) {
116+
if (!$element->hasId()) continue;
115117
$elementMap[$element->getId()] = $element;
116118

117119
if ($element->hasValue() && array_key_exists($idx, $raw)) {
118120
$idMap[$element->getId()] = $raw[$idx];
121+
122+
if ($element instanceof ValidatedInput) {
123+
if (!$element->validate((string) $raw[$idx])) {
124+
throw new FormValidationException($element->getErrorMessage());
125+
}
126+
}
119127
}
120128
}
121129
}
@@ -132,7 +140,7 @@ public function handleResponse(Player $player, $raw): void {
132140

133141
$valueElementCount = count(array_filter($elements, fn($e) => $e->hasValue()));
134142
if (!$this->force && count($idMap) !== $valueElementCount) {
135-
throw new InvalidArgumentException("Invalid number of fields.");
143+
throw new FormValidationException("Invalid number of fields.");
136144
}
137145

138146
$result = $this->onSubmit($player, $response);
@@ -149,7 +157,11 @@ public function handleResponse(Player $player, $raw): void {
149157
* @return array Serialized data
150158
*/
151159
protected function serializeFormData(): array {
152-
$elements = $this->getCachedElements($this->player, $this->formData);
160+
$player = $this->playerRef->get();
161+
if ($player === null) {
162+
return ['content' => []];
163+
}
164+
$elements = $this->getCachedElements($player, $this->formData);
153165
return [
154166
'content' => array_map(
155167
fn(JsonSerializable $e) => $e->jsonSerialize(),
@@ -168,22 +180,15 @@ protected function getFormType(): string {
168180
}
169181

170182
/**
171-
* Gets and validates cached elements.
183+
* Gets cached elements.
172184
*
173185
* @param Player $player Viewing player
174186
* @param FormData $data Form data
175-
* @return array Validated elements
176-
* @throws InvalidArgumentException If elements lack IDs
187+
* @return array Elements array
177188
*/
178189
private function getCachedElements(Player $player, FormData $data): array {
179190
if ($this->elementsCache === null) {
180191
$this->elementsCache = $this->elements($player, $data);
181-
182-
foreach ($this->elementsCache as $element) {
183-
if (!$element->hasId()) {
184-
throw new InvalidArgumentException("All elements must have an ID.");
185-
}
186-
}
187192
}
188193
return iterator_to_array($this->elementsCache);
189194
}

‎src/imperazim/form/custom/DynamicCustomForm.php‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
use Closure;
88
use JsonSerializable;
99
use pocketmine\player\Player;
10+
use pocketmine\form\FormValidationException;
1011
use imperazim\form\Form;
1112
use imperazim\form\FormData;
1213
use imperazim\form\FormResult;
1314
use imperazim\form\base\elements\Title;
1415
use imperazim\form\custom\elements\Element;
1516
use imperazim\form\custom\elements\ElementCollection;
17+
use imperazim\form\custom\elements\ValidatedInput;
1618
use imperazim\form\custom\response\CustomResponse;
1719

1820
/**
@@ -46,7 +48,7 @@ public function __construct(Title $title) {
4648
* @param string|Title $title Form title
4749
* @return self New instance
4850
*/
49-
final public static function create(string|Title $title): self {
51+
public static function create(string|Title $title): self {
5052
return new self($title instanceof Title ? $title : new Title($title));
5153
}
5254

@@ -57,7 +59,7 @@ final public static function create(string|Title $title): self {
5759
* @param Closure $configurator Configuration callback
5860
* @return self Configured instance
5961
*/
60-
final public static function build(string|Title $title, Closure $configurator): self {
62+
public static function build(string|Title $title, Closure $configurator): self {
6163
$form = self::create($title);
6264
$configurator($form);
6365
return $form;
@@ -125,25 +127,36 @@ protected function getFormType(): string {
125127
* @param Player $player Responding player
126128
* @param mixed $raw Raw response data
127129
*/
128-
public function handleResponse(Player $player, $raw): void {
130+
public function handleResponse(Player $player, mixed $raw): void {
129131
if ($raw === null) {
130132
$result = ($this->onClose)($player);
131133
} elseif (is_array($raw)) {
134+
$expectedCount = count(iterator_to_array($this->elements));
135+
if (count($raw) !== $expectedCount) {
136+
throw new FormValidationException("Expected $expectedCount fields, got " . count($raw));
137+
}
132138
$idMap = [];
133139
$elementMap = [];
134140

135141
foreach ($this->elements as $idx => $element) {
142+
if (!$element->hasId()) continue;
136143
$elementMap[$element->getId()] = $element;
137144

138145
if ($element->hasValue() && array_key_exists($idx, $raw)) {
139146
$idMap[$element->getId()] = $raw[$idx];
147+
148+
if ($element instanceof ValidatedInput) {
149+
if (!$element->validate((string) $raw[$idx])) {
150+
throw new FormValidationException($element->getErrorMessage());
151+
}
152+
}
140153
}
141154
}
142155

143156
$response = new CustomResponse(new FormData([]), $raw, $idMap, $elementMap);
144157
$result = ($this->onSubmit)($player, $response);
145158
} else {
146-
throw new InvalidArgumentException("Invalid response data type");
159+
throw new FormValidationException("Invalid response data type");
147160
}
148161

149162
if ($result === FormResult::KEEP) {

‎src/imperazim/form/custom/elements/Dropdown.php‎

Lines changed: 4 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
* Represents a dropdown selection element.
1111
*/
1212
final class Dropdown extends Element {
13+
use OptionNormalizerTrait;
14+
1315
/** @var Option[] Available options */
1416
private array $options;
1517

@@ -23,7 +25,7 @@ final class Dropdown extends Element {
2325
* @param string|int $default Default selection
2426
*/
2527
public function __construct(
26-
private string $id,
28+
string $id,
2729
private string $text,
2830
array $options,
2931
string|int $default = 0
@@ -33,48 +35,6 @@ public function __construct(
3335
$this->defaultIndex = $this->findOptionIndex($default);
3436
}
3537

36-
/**
37-
* Normalizes options array.
38-
*
39-
* @param array $options Raw options
40-
* @return Option[] Normalized options
41-
*/
42-
private function normalizeOptions(array $options): array {
43-
$normalized = [];
44-
foreach ($options as $key => $value) {
45-
if ($value instanceof Option) {
46-
$normalized[] = $value;
47-
} elseif (is_array($value)) {
48-
$normalized[] = new Option(
49-
(string)($value['id'] ?? $key),
50-
(string)($value['text'] ?? '')
51-
);
52-
} else {
53-
$normalized[] = new Option((string)$key, (string)$value);
54-
}
55-
}
56-
return $normalized;
57-
}
58-
59-
/**
60-
* Finds option index by ID or value.
61-
*
62-
* @param string|int $default Default identifier
63-
* @return int Option index
64-
*/
65-
private function findOptionIndex(string|int $default): int {
66-
if (is_int($default)) {
67-
return $default;
68-
}
69-
70-
foreach ($this->options as $index => $option) {
71-
if ($option->getId() === $default) {
72-
return $index;
73-
}
74-
}
75-
return 0;
76-
}
77-
7838
/**
7939
* Serializes element for JSON.
8040
*
@@ -88,33 +48,4 @@ public function jsonSerialize(): array {
8848
'default' => $this->defaultIndex
8949
];
9050
}
91-
92-
/**
93-
* Gets option by index.
94-
*
95-
* @param int $index Option position
96-
* @return Option|null Option or null
97-
*/
98-
public function getOptionByIndex(int $index): ?Option {
99-
return $this->options[$index] ?? null;
100-
}
101-
102-
/**
103-
* Gets selected option.
104-
*
105-
* @param int $index Selected index
106-
* @return SelectedOption|null Selected option or null
107-
*/
108-
public function getSelectedOption(int $index): ?SelectedOption {
109-
$option = $this->getOptionByIndex($index);
110-
if ($option === null) {
111-
return null;
112-
}
113-
114-
return new SelectedOption(
115-
$index,
116-
$option->getId(),
117-
$option->getText()
118-
);
119-
}
120-
}
51+
}

‎src/imperazim/form/custom/elements/Input.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ final class Input extends Element {
1515
* @param string $default Default value
1616
*/
1717
public function __construct(
18-
private string $id,
18+
string $id,
1919
private string $text,
2020
private string $placeholder = '',
2121
private string $default = ''

0 commit comments

Comments
 (0)