Skip to content

Commit 375a5cb

Browse files
committed
Add orderBy/limit to GetAllExpressionsWithMetadata and validate in UI
- Add orderBy and limit expressions (type "number") to ForEachEvent::GetAllExpressionsWithMetadata so refactoring propagates to them. - Expose GetOrderByExpression/GetLimitExpression in C++ header, IDL bindings, and JS types for expression validation. - Show red underlines (instructionInvalidParameter) on invalid orderBy and limit expressions in the ForEachEvent renderer, matching the RepeatEvent pattern. Empty limit is treated as valid. - Replace cursor:pointer on inline selects with the "selectable" CSS class for consistent styling. https://claude.ai/code/session_01LXTeNz7ehNS5TqFYSU6aw5
1 parent 92fea14 commit 375a5cb

File tree

6 files changed

+80
-9
lines changed

6 files changed

+80
-9
lines changed

Core/GDCore/Events/Builtin/ForEachEvent.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,14 @@ vector<pair<gd::Expression*, gd::ParameterMetadata> >
3939
ForEachEvent::GetAllExpressionsWithMetadata() {
4040
vector<pair<gd::Expression*, gd::ParameterMetadata> >
4141
allExpressionsWithMetadata;
42-
auto metadata = gd::ParameterMetadata().SetType("object");
42+
auto objectMetadata = gd::ParameterMetadata().SetType("object");
4343
allExpressionsWithMetadata.push_back(
44-
std::make_pair(&objectsToPick, metadata));
44+
std::make_pair(&objectsToPick, objectMetadata));
45+
auto numberMetadata = gd::ParameterMetadata().SetType("number");
46+
allExpressionsWithMetadata.push_back(
47+
std::make_pair(&orderBy, numberMetadata));
48+
allExpressionsWithMetadata.push_back(
49+
std::make_pair(&limit, numberMetadata));
4550

4651
return allExpressionsWithMetadata;
4752
}
@@ -65,9 +70,14 @@ vector<pair<const gd::Expression*, const gd::ParameterMetadata> >
6570
ForEachEvent::GetAllExpressionsWithMetadata() const {
6671
vector<pair<const gd::Expression*, const gd::ParameterMetadata> >
6772
allExpressionsWithMetadata;
68-
auto metadata = gd::ParameterMetadata().SetType("object");
73+
auto objectMetadata = gd::ParameterMetadata().SetType("object");
74+
allExpressionsWithMetadata.push_back(
75+
std::make_pair(&objectsToPick, objectMetadata));
76+
auto numberMetadata = gd::ParameterMetadata().SetType("number");
77+
allExpressionsWithMetadata.push_back(
78+
std::make_pair(&orderBy, numberMetadata));
6979
allExpressionsWithMetadata.push_back(
70-
std::make_pair(&objectsToPick, metadata));
80+
std::make_pair(&limit, numberMetadata));
7181

7282
return allExpressionsWithMetadata;
7383
}

Core/GDCore/Events/Builtin/ForEachEvent.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class GD_CORE_API ForEachEvent : public gd::BaseEvent {
6565
void SetOrderBy(gd::String orderBy_) {
6666
orderBy = gd::Expression(orderBy_);
6767
};
68+
const gd::Expression& GetOrderByExpression() const { return orderBy; };
6869

6970
const gd::String& GetOrder() const { return order; }
7071
void SetOrder(const gd::String& order_) { order = order_; }
@@ -75,6 +76,7 @@ class GD_CORE_API ForEachEvent : public gd::BaseEvent {
7576
void SetLimit(gd::String limit_) {
7677
limit = gd::Expression(limit_);
7778
};
79+
const gd::Expression& GetLimitExpression() const { return limit; };
7880

7981
virtual std::vector<const gd::InstructionsList*> GetAllConditionsVectors()
8082
const;

GDevelop.js/Bindings/Bindings.idl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2520,10 +2520,12 @@ interface ForEachEvent {
25202520
void SetLoopIndexVariableName([Const] DOMString name);
25212521
[Const, Ref] DOMString GetOrderBy();
25222522
void SetOrderBy([Const] DOMString orderBy);
2523+
[Const, Ref] Expression GetOrderByExpression();
25232524
[Const, Ref] DOMString GetOrder();
25242525
void SetOrder([Const] DOMString order);
25252526
[Const, Ref] DOMString GetLimit();
25262527
void SetLimit([Const] DOMString limit);
2528+
[Const, Ref] Expression GetLimitExpression();
25272529
};
25282530
ForEachEvent implements BaseEvent;
25292531

GDevelop.js/types.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1942,10 +1942,12 @@ export class ForEachEvent extends BaseEvent {
19421942
setLoopIndexVariableName(name: string): void;
19431943
getOrderBy(): string;
19441944
setOrderBy(orderBy: string): void;
1945+
getOrderByExpression(): Expression;
19451946
getOrder(): string;
19461947
setOrder(order: string): void;
19471948
getLimit(): string;
19481949
setLimit(limit: string): void;
1950+
getLimitExpression(): Expression;
19491951
}
19501952

19511953
export class ForEachChildVariableEvent extends BaseEvent {

GDevelop.js/types/gdforeachevent.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ declare class gdForEachEvent extends gdBaseEvent {
99
setLoopIndexVariableName(name: string): void;
1010
getOrderBy(): string;
1111
setOrderBy(orderBy: string): void;
12+
getOrderByExpression(): gdExpression;
1213
getOrder(): string;
1314
setOrder(order: string): void;
1415
getLimit(): string;
1516
setLimit(limit: string): void;
17+
getLimitExpression(): gdExpression;
1618
delete(): void;
1719
ptr: number;
1820
};

newIDE/app/src/EventsSheet/EventsTree/Renderers/ForEachEvent.js

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ const styles = {
5353
background: 'none',
5454
border: 'none',
5555
borderBottom: '1px dashed currentColor',
56-
cursor: 'pointer',
5756
fontSize: 'inherit',
5857
fontFamily: 'inherit',
5958
color: 'inherit',
@@ -223,11 +222,39 @@ export default class ForEachEvent extends React.Component<
223222
render(): any {
224223
const forEachEvent = gd.asForEachEvent(this.props.event);
225224
const objectName = forEachEvent.getObjectToPick();
226-
const orderBy = forEachEvent.getOrderBy();
225+
const orderByExpression = forEachEvent.getOrderByExpression();
226+
const orderBy = orderByExpression.getPlainString();
227227
const order = forEachEvent.getOrder();
228-
const limit = forEachEvent.getLimit();
228+
const limitExpression = forEachEvent.getLimitExpression();
229+
const limit = limitExpression.getPlainString();
229230
const hasOrderBy = !!orderBy;
230231

232+
let isOrderByValid = true;
233+
if (hasOrderBy) {
234+
const orderByValidator = new gd.ExpressionValidator(
235+
gd.JsPlatform.get(),
236+
this.props.projectScopedContainersAccessor.get(),
237+
'number',
238+
''
239+
);
240+
orderByExpression.getRootNode().visit(orderByValidator);
241+
isOrderByValid = orderByValidator.getAllErrors().size() === 0;
242+
orderByValidator.delete();
243+
}
244+
245+
let isLimitValid = true;
246+
if (limit) {
247+
const limitValidator = new gd.ExpressionValidator(
248+
gd.JsPlatform.get(),
249+
this.props.projectScopedContainersAccessor.get(),
250+
'number',
251+
''
252+
);
253+
limitExpression.getRootNode().visit(limitValidator);
254+
isLimitValid = limitValidator.getAllErrors().size() === 0;
255+
limitValidator.delete();
256+
}
257+
231258
const objectNameIsValid = this.props.projectScopedContainersAccessor
232259
.get()
233260
.getObjectsContainersList()
@@ -298,6 +325,7 @@ export default class ForEachEvent extends React.Component<
298325
</Trans>{' '}
299326
{/* Inline select for "ordered by" vs "(any order)" */}
300327
<select
328+
className={selectableArea}
301329
style={styles.inlineSelect}
302330
value={hasOrderBy ? 'orderBy' : 'any'}
303331
onChange={e => {
@@ -336,10 +364,21 @@ export default class ForEachEvent extends React.Component<
336364
}}
337365
tabIndex={0}
338366
>
339-
{orderBy}
367+
{isOrderByValid ? (
368+
orderBy
369+
) : (
370+
<span
371+
className={classNames({
372+
[instructionInvalidParameter]: true,
373+
})}
374+
>
375+
{orderBy}
376+
</span>
377+
)}
340378
</span>
341379
({/* Inline select for ascending/descending */}
342380
<select
381+
className={selectableArea}
343382
style={styles.inlineSelect}
344383
value={order}
345384
onChange={e => {
@@ -371,7 +410,21 @@ export default class ForEachEvent extends React.Component<
371410
}}
372411
tabIndex={0}
373412
>
374-
{limit || <Trans>no limit</Trans>}
413+
{limit ? (
414+
isLimitValid ? (
415+
limit
416+
) : (
417+
<span
418+
className={classNames({
419+
[instructionInvalidParameter]: true,
420+
})}
421+
>
422+
{limit}
423+
</span>
424+
)
425+
) : (
426+
<Trans>no limit</Trans>
427+
)}
375428
</span>
376429
</span>
377430
)

0 commit comments

Comments
 (0)