Zoning UI cleanup 2 #179
Zoning UI cleanup 2 #179
Conversation
There was a problem hiding this comment.
I know this is still a draft but figured I'd have a quick skim over this. If you're having issues with null/undefined stuff I'd be happy to go over best practices more, they can be a real bastard to work with.
Could also enable TS strict mode specifically for this file, which will yell at you like crazy at first, but once you fix all the strict null check issues it can make getting null-checking
right much easier.
| <include type="module" src="file://{scripts}/pages/zoning/zoning.ts" /> | ||
| </scripts> | ||
|
|
||
| </root> |
| this.createTrackEntry(this.panels.trackList, this.mapZoneData.tracks.main, $.Localize('#Zoning_Main')); | ||
|
|
||
| // if the first segment has no start zone, show text to create start zone until a start zone has been made | ||
| if (this.mapZoneData.tracks.main.zones.segments[0]?.checkpoints.length === 0) { | ||
| if (this.mapZoneData.tracks.main.zones?.segments?.length > 0 && this.mapZoneData.tracks.main.zones?.segments[0]?.checkpoints?.length) { |
There was a problem hiding this comment.
Don't think you need this length check. zones?.segments[0]?. is null if array is length 0, which ?.s carry to the end of the expression, so the final .length is either a number or null
| zone: null | ||
| } | ||
| ); | ||
| if (track.zones?.segments?.length > 0) { |
There was a problem hiding this comment.
Shouldn't need this check. If you're worried about the array being null, you can do for (const [i, segment] of track.zones.segments?.entries() ?? []). Looks a bit weird, but saves a whole level of indentation.
| }); | ||
| } | ||
| const segmentCheckpointContainer = segmentChildContainer.FindChildTraverse<Panel>('CheckpointContainer'); | ||
| if (segment?.checkpoints?.length > 0) { |
| zone: zone | ||
| }); | ||
| const segmentCancelContainer = segmentChildContainer.FindChildTraverse<Panel>('CancelContainer'); | ||
| if (segment?.cancel?.length > 0) { |
| for (const [i, item] of array?.entries()) { | ||
| this.addOptionToDropdown(optionType || item, dropdown, i, optionType !== ''); | ||
| } |
There was a problem hiding this comment.
This I don't think is safe, however. If array is null, your elvis will propagate it along, and you'll get a JS exception trying to iterate over null.
| this.panels.regionBottom.text = (region?.bottom ?? 0).toFixed(2); | ||
| this.panels.regionHeight.text = (region?.height ?? 0).toFixed(2); | ||
| this.panels.regionSafeHeight.text = (region?.safeHeight ?? 0).toFixed(2); | ||
|
|
||
| const tpIndex = !region.teleDestTargetname | ||
| const tpIndex = (region.teleDestTargetname === undefined || region.teleDestTargetname === '') |
There was a problem hiding this comment.
Empty string (and only the empty string) is falsy so original seems fine? Unless you're concerned about it being null, in which case I'd maybe rethink this code since combining undefined and null like this tends to lead to very confusing code.
| editing: validity.zone && region === this.selectedZone.zone.regions[regionIndex] | ||
| }); | ||
|
|
||
| if (this.selectedZone.track.zones?.segments?.length > 0) { |
There was a problem hiding this comment.
Again you can save a bunch of indents using by skipping the length checks. Could also do some psycho flatMaps (though this is untested and absolutely not required)
const regionRegions =
this.selectedZone.track.zones?.segments?.flatMap((segment, segmentNumber) => [
...(segment?.checkpoints?.flatMap((checkpoint, checkpointNumber) =>
checkpoint.regions.map((region) => ({
region: region,
renderMode:
checkpointNumber === 0
? segmentNumber === 0
? RegionRenderMode.START
: RegionRenderMode.MAJOR_CHECKPOINT
: RegionRenderMode.MINOR_CHECKPOINT,
editing: validity.zone && region === this.selectedZone.region,
})),
) ?? []),
...(segment?.cancel?.flatMap((cancel) =>
cancel.regions.map((region) => ({
region: region,
renderMode: RegionRenderMode.CANCEL,
editing: validity.zone && region === this.selectedZone.region,
})),
) ?? []),
]) ?? [];while zoning, communicate between engine and frontend with whole region instead of individual points
PeenScreeker
force-pushed
the
feat/zoning-ui-cleanup-2
branch
from
4989b80 to
fa7f9d4
Compare
Closes momentum-mod/game/issues/
This is a draft I'm opening so that red changes can be tested during review if anyone wants to try them out.
Checks
feat: Add foo,chore: Update bar, etc...fixuped into my original commits.