Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for per scene and project wide DMG palettes #1601

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0c5dc0b
dmg palettes (BGP, OBP0, OBP1) assignable in project settings file (n…
Q-Bert-Reynolds Oct 1, 2024
27c9f71
stubbs out settings page for DMG palettes
Q-Bert-Reynolds Oct 1, 2024
a840e83
functional but ugly... how the fuck do you change the color of a UI e…
Q-Bert-Reynolds Oct 2, 2024
feb87db
ok, that looks decent... now for scenes... and I should probably make…
Q-Bert-Reynolds Oct 2, 2024
5c1579b
obj pal select shows configured palette
Q-Bert-Reynolds Oct 2, 2024
8af70eb
can now see custom BGP in scene view
Q-Bert-Reynolds Oct 2, 2024
b979450
OBP palette selection shows on sprites... something seems off though
Q-Bert-Reynolds Oct 2, 2024
e65305e
fixes issue with incorrect sprite palletization
Q-Bert-Reynolds Oct 2, 2024
491cc10
fixes issue where preview as mono showed wrong sprite pallete in scen…
Q-Bert-Reynolds Oct 2, 2024
6f01ceb
fixes issue where switching between mono and color wouldn't update sp…
Q-Bert-Reynolds Oct 2, 2024
0e8ac08
adds preview as monochrome option to sprite and image pages
Q-Bert-Reynolds Oct 2, 2024
ebbad21
adds dmg palette picker to tidy up settings page and prepare for addi…
Q-Bert-Reynolds Oct 2, 2024
adfdd17
adds dmg palette pickers to scene editor, fixes issue with picker tha…
Q-Bert-Reynolds Oct 3, 2024
a5b470a
better looking... maybe
Q-Bert-Reynolds Oct 3, 2024
8cd7007
uglier code, prettier editor
Q-Bert-Reynolds Oct 4, 2024
dbb4514
updates sprite colorization to be more consistent with current user e…
Q-Bert-Reynolds Oct 4, 2024
ffcbf8d
add MonoPalette type instead of using [number,number,number,number] e…
Q-Bert-Reynolds Oct 5, 2024
7a59195
fixes issue where scene actors weren't updating their colors when the…
Q-Bert-Reynolds Oct 5, 2024
3237f33
replaces hard-coded UI text with language keys for english and spanish
Q-Bert-Reynolds Oct 5, 2024
591b584
fixes regression where color sprites had incorrect palettes in editor
Q-Bert-Reynolds Oct 5, 2024
53209d2
actor and scene editor sprites now display in color
Q-Bert-Reynolds Oct 6, 2024
736c0da
background select now uses scene colors where appropriate
Q-Bert-Reynolds Oct 6, 2024
1df68d9
giving up on colorized backgrounds, they work but I have no idea how …
Q-Bert-Reynolds Oct 7, 2024
d334907
small refactor of react hooks... I think this is better
Q-Bert-Reynolds Oct 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/components/backgrounds/BackgroundPreviewSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
backgroundSelectors,
sceneSelectors,
} from "store/features/entities/entitiesState";
import settingsActions from "store/features/settings/settingsActions";
import editorActions from "store/features/editor/editorActions";
import electronActions from "store/features/electron/electronActions";
import { SceneSelect } from "components/forms/SceneSelect";
Expand Down Expand Up @@ -66,7 +67,12 @@ const BackgroundPreviewSettings = ({
);
const [isOpen, setIsOpen] = useState<boolean>(false);
const [buttonFocus, setButtonFocus] = useState<boolean>(false);
const value = useAppSelector((state) => state.editor.previewAsSceneId);
const value = useAppSelector((state) => {
if (state.project.present.settings.previewAsMono) return "0";
const sceneId = state.editor.previewAsSceneId;
if (sceneId === "") return "1";
return sceneId;
});
Comment on lines +70 to +75
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a mono mode, similar to the one found on the world page, to the "preview as" drop down. Previously, only one non-scene option was available in this drop down, "Default Colors", which was used when previewAsSceneId was "". To facilitate more options, I'm just using the stringified index of the non-scene values. To ensure a consistent experience when switching between pages, I set the default state to "0" when previewAsMono and "1" when previewAsSceneId is unset.

const scene = useAppSelector((state) =>
sceneSelectors.selectById(state, value)
);
Expand Down Expand Up @@ -144,6 +150,11 @@ const BackgroundPreviewSettings = ({
};

const onChange = (newValue: string) => {
dispatch(
settingsActions.editSettings({
previewAsMono: (newValue == "0"),
})
);
dispatch(editorActions.setPreviewAsSceneId(newValue));
};
Comment on lines 152 to 159
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also using the "preview as" drop down selection to drive the previewAsMono editor setting so if you select it here or in the sprites page, the scenes will be in monochrome when you return to the world page.


Expand Down Expand Up @@ -173,7 +184,7 @@ const BackgroundPreviewSettings = ({
onBlur={closeMenu}
maxMenuHeight={200}
optional
optionalLabel={l10n("FIELD_DEFAULT_COLORS")}
optionalLabels={[l10n("FIELD_COLOR_MODE_MONO"), l10n("FIELD_DEFAULT_COLORS")]}
{...selectMenuStyleProps}
/>
</SelectMenu>
Expand All @@ -190,7 +201,8 @@ const BackgroundPreviewSettings = ({
? l10n("FIELD_PREVIEW_AS_SCENE", {
sceneName: sceneName(scene, sceneIndex),
})
: l10n("FIELD_PREVIEW_AS_DEFAULT")}
: ([l10n("FIELD_PREVIEW_AS_MONO"), l10n("FIELD_PREVIEW_AS_DEFAULT")][+value])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As someone who has never used TypeScript before, I found the [+value] syntax a little strange, but it's what StackOverflow told me to do. I'm considering revisiting the weird stringified index hack when I add SGB preview support anyway, but this is a little too hacky for your tastes now, let me know.

}
</Pill>
<FixedSpacer width={5} />
</>
Expand Down
12 changes: 11 additions & 1 deletion src/components/backgrounds/BackgroundViewer.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useState } from "react";
import React, { useCallback, useEffect, useState } from "react";
import { useAppSelector } from "store/hooks";
import styled from "styled-components";
import {
Expand Down Expand Up @@ -84,6 +84,12 @@ const BackgroundViewer = ({ backgroundId }: MetaspriteEditorProps) => {
(state) => state.project.present.settings.colorMode !== "mono"
);
const [palettes, setPalettes] = useState<Palette[]>(emptyPalettes);
const previewAsMono = useAppSelector((state) =>
!gbcEnabled || state.project.present.settings.previewAsMono
);
const defaultBGP = useAppSelector((state) =>
state.project.present.settings.defaultBGP
);

useEffect(() => {
setPalettes(
Expand Down Expand Up @@ -128,6 +134,8 @@ const BackgroundViewer = ({ backgroundId }: MetaspriteEditorProps) => {
src={assetURL("backgrounds", background)}
tiles={background.tileColors}
palettes={palettes}
previewAsMono={previewAsMono}
monoPalette={defaultBGP}
/>
)}
</ImageScale>
Expand Down Expand Up @@ -162,6 +170,8 @@ const BackgroundViewer = ({ backgroundId }: MetaspriteEditorProps) => {
src={assetURL("tilesets", tileset)}
tiles={[]}
palettes={palettes}
previewAsMono={previewAsMono}
monoPalette={defaultBGP}
/>
</ImageScale>
</ImageContainer>
Expand Down
72 changes: 65 additions & 7 deletions src/components/editors/SceneEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import { ScriptEditorCtx } from "shared/lib/scripts/context";
import { TilesetSelect } from "components/forms/TilesetSelect";
import { FlexGrow } from "ui/spacing/Spacing";
import CachedScroll from "ui/util/CachedScroll";
import { DMGPalettePicker } from "components/forms/DMGPalettePicker";

interface SceneEditorProps {
id: string;
Expand Down Expand Up @@ -149,7 +150,18 @@ export const SceneEditor = ({ id }: SceneEditorProps) => {
const [commonTilesetOpen, setCommonTilesetOpen] = useState<boolean>(
!!scene?.tilesetId
);

const defaultBGP = useAppSelector((state) =>
state.project.present.settings.defaultBGP
);
const defaultOBP0 = useAppSelector((state) =>
state.project.present.settings.defaultOBP0
);
const defaultOBP1 = useAppSelector((state) =>
state.project.present.settings.defaultOBP1
);
const monoEnabled = useAppSelector(
(state) => state.project.present.settings.colorMode !== "color"
);
const colorsEnabled = useAppSelector(
(state) => state.project.present.settings.colorMode !== "mono"
);
Expand Down Expand Up @@ -324,6 +336,24 @@ export const SceneEditor = ({ id }: SceneEditorProps) => {
[onChangeSettingProp]
);

const onEditBGP = useCallback(
(palette: [number, number, number, number]) =>
onChangeSceneProp("dmgBGP", palette),
[onChangeSceneProp]
);

const onEditOBP0 = useCallback(
(palette: [number, number, number, number]) =>
onChangeSceneProp("dmgOBP0", palette),
[onChangeSceneProp]
);

const onEditOBP1 = useCallback(
(palette: [number, number, number, number]) =>
onChangeSceneProp("dmgOBP1", palette),
[onChangeSceneProp]
);

const selectSidebar = () => {
dispatch(editorActions.selectSidebar());
};
Expand Down Expand Up @@ -739,14 +769,14 @@ export const SceneEditor = ({ id }: SceneEditorProps) => {
</SidebarColumn>
)}

{colorsEnabled && (
<SidebarColumn>
<FormRow>
<FormField
name="playerSpriteSheetId"
label={
<>
{l10n("FIELD_SCENE_BACKGROUND_PALETTES")}
Comment on lines -742 to -749
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that there's a palette to edit even in monochrome mode, I moved the colorsEnabled check after the label. I had a strong urge to update the indentation everywhere, but I left it alone to avoid a massive unreadable diff. There are a few bits that looked too odd without any indentation changes, but it's probably not a bad idea to fully correct the indentation levels for consistency after the bulk of the code review is done.

{l10n("FIELD_SCENE_BACKGROUND_PALETTES")}
{colorsEnabled && (
<InlineDropdownWrapper>
<DropdownButton
size="small"
Expand Down Expand Up @@ -784,10 +814,19 @@ export const SceneEditor = ({ id }: SceneEditorProps) => {
</MenuItem>
</DropdownButton>
</InlineDropdownWrapper>
)}
</>
}
>
{!background?.autoColor && (
}>
{monoEnabled && (
<DMGPalettePicker
name="bgp"
palette={scene.dmgBGP ?? defaultBGP}
isSpritePalette={false}
onChange={onEditBGP}
showName={true}
/>
)}
{colorsEnabled && !background?.autoColor && (
<PaletteButtons>
{[0, 1, 2, 3, 4, 5, 6, 7].map((index) => (
<PaletteSelectButton
Expand Down Expand Up @@ -816,6 +855,25 @@ export const SceneEditor = ({ id }: SceneEditorProps) => {
name="playerSpriteSheetId"
label={l10n("FIELD_SCENE_SPRITE_PALETTES")}
>
{monoEnabled &&
<PaletteButtons>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps PaletteButtons was the wrong wrapper node to use here, but I used it as a quick and dirty way of keeping the OBP0 and OBP1 palette pickers on the same row.

<DMGPalettePicker
name="obp0"
palette={scene.dmgOBP0 ?? defaultOBP0}
isSpritePalette={true}
onChange={onEditOBP0}
showName={true}
/>
<DMGPalettePicker
name="obp1"
palette={scene.dmgOBP1 ?? defaultOBP1}
isSpritePalette={true}
onChange={onEditOBP1}
showName={true}
/>
</PaletteButtons>
}
{colorsEnabled && (
<PaletteButtons>
{[0, 1, 2, 3, 4, 5, 6, 7].map((index) => (
<PaletteSelectButton
Expand All @@ -837,12 +895,12 @@ export const SceneEditor = ({ id }: SceneEditorProps) => {
/>
))}
</PaletteButtons>
)}
</FormField>
</FormRow>

{/* <FormDivider /> */}
</SidebarColumn>
)}
{scene.type !== "LOGO" && (
<SidebarColumn>
<FormRow>
Expand Down
61 changes: 61 additions & 0 deletions src/components/forms/DMGPalettePicker.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import React, { useCallback } from "react";
import {useAppSelector } from "store/hooks";
import { NumberInput } from "ui/form/NumberInput";
import { castEventToInt } from "renderer/lib/helpers/castEventValue";
import styled, { css } from "styled-components";

type DMGPalettePickerProps = {
name: string
palette: [number, number, number, number]
isSpritePalette: boolean
onChange: (palette: [number, number, number, number]) => void;
showName?: boolean
};

export const DMGPalettePicker = ({
name,
palette,
isSpritePalette,
onChange,
showName
}: DMGPalettePickerProps) => {

const settings = useAppSelector((state) => state.project.present.settings);
const dmgColors = [settings.customColorsWhite, settings.customColorsLight, settings.customColorsDark, settings.customColorsBlack];
const fields = isSpritePalette ? [1, 2, 3] : [0, 1, 2, 3];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At one point, I was showing all 4 input boxes for sprites. Then, I realized the lowest two bits are never used anyway and it felt silly to show them. I didn't want to change the underlying data structure from [number, number, number, number] to number[] though because dealing with arrays of different sizes complicated the logic further and didn't line up as well with the byte that it represented in a build.

const width = showName ? `max-width: ${isSpritePalette ? 121 : 245}px;` : "width: 100%";
const DMGPalettePickerStyle = styled.div`
display: flex;
${width}
box-sizing: border-box;
`

const DMGPaletteTextStyle = styled.div`
width: 9px;
color: grey;
font-size: 9px;
text-align: center;
transform-origin:50% 50%;
transform: rotate(90deg) translate(-7px,8px);
Comment on lines +27 to +40
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of hacky stuff going on with the styling:

  • I wanted the pickers to be full width in the settings screen, but only as wide as the color palette buttons and for OBP0 and OBP1 to be on the same row in the scene editor.
  • I wanted there to be text labels in the scene editor, but I wanted them to be tiny, greyed out, unobtrusive, and vertical.
  • I killed off the spacing between numerical inputs so the elements look more like they belong to each other, but it makes the red selection outline look a little funky since it expects a 5px spacing.
  • In the getRows function below, I explicitly set the background and text colors from the default color palette. Didn't know of a cleaner way of doing that. Also, the text color seems fine, but using an offset index to lookup the palette color may be problematic in the future if the default DMG palette is ever configurable.
  • I also changed the min size of NumberInput to allow both sprite palettes to fit on a single row and only be as wide as the palette buttons below it. I didn't notice any issues, but if there's a way to override the styling of child elements instead of just changing the min-width globally, I'd rather do that.

`
function getRows() {
return fields.map((index) =>
<NumberInput
style={{backgroundColor:`#${dmgColors[palette[index]]}`, color:`#${dmgColors[(palette[index]+2)%4]}`}}
id={`${name}_${index}`}
name={`${name}_${index}`}
min={0}
max={3}
value={palette[index]}
onChange={(e) => {
let newPalette = [palette[0], palette[1], palette[2], palette[3]];
newPalette[index] = castEventToInt(e, 0);
onChange(newPalette as [number, number, number, number]);
}}
/>
);
}

if (showName) return <DMGPalettePickerStyle><DMGPaletteTextStyle>{name.toUpperCase()}</DMGPaletteTextStyle>{getRows()}</DMGPalettePickerStyle>;
else return <DMGPalettePickerStyle>{getRows()}</DMGPalettePickerStyle>;
};
33 changes: 20 additions & 13 deletions src/components/forms/ObjPaletteSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from "ui/form/Select";
import l10n from "shared/lib/lang/l10n";
import { SingleValue } from "react-select";
import { useAppSelector } from "store/hooks";

interface ObjPaletteSelectProps {
name: string;
Expand All @@ -21,24 +22,30 @@ interface ObjPaletteOption {
colors: string[];
}

const options: ObjPaletteOption[] = [
{
value: "OBP0",
label: "0: OBP0",
colors: ["E8F8E0", "B0F088", "", "202850"],
},
{
value: "OBP1",
label: "1: OBP1",
colors: ["E8F8E0", "509878", "", "202850"],
},
];

export const ObjPaletteSelect: FC<ObjPaletteSelectProps> = ({
name,
value = "OBP0",
onChange,
}) => {
const settings = useAppSelector((state) => state.project.present.settings);
const obp0 = settings.defaultOBP0;
const obp1 = settings.defaultOBP1;
const dmgPal = [settings.customColorsWhite, settings.customColorsLight, settings.customColorsDark, settings.customColorsBlack];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the custom colors from settings instead of DMG_PALETTE.colors because I thought for sure I'd be making those configurable in this PR. I still plan on doing that, but I realized how cumbersome the task was when the various workers complained about trying to access settings off the main thread, so I abandoned ship there.

const obp0Colors = [dmgPal[obp0[1]], dmgPal[obp0[2]], "", dmgPal[obp0[3]]];
const obp1Colors = [dmgPal[obp1[1]], dmgPal[obp1[2]], "", dmgPal[obp1[3]]];
const options: ObjPaletteOption[] = [
{
value: "OBP0",
label: "0: OBP0",
colors: obp0Colors,
},
{
value: "OBP1",
label: "1: OBP1",
colors: obp1Colors,
},
];

const currentValue = options.find((o) => o.value === value);
return (
<Select
Expand Down
23 changes: 7 additions & 16 deletions src/components/forms/SceneSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ interface SceneSelectProps extends SelectCommonProps {
name: string;
value?: string;
optional?: boolean;
optionalLabel?: string;
optionalLabels?: string[];
onChange?: (newId: string) => void;
}

Expand Down Expand Up @@ -61,10 +61,11 @@ export const SceneSelect: FC<SceneSelectProps> = ({
value,
onChange,
optional,
optionalLabel,
optionalLabels,
...selectProps
}) => {
const scenes = useAppSelector((state) => sceneSelectors.selectAll(state));
const optionalScenes = (optionalLabels || []).map((optionalLabel, i) => ({ value: i.toString(), label: optionalLabel }) ) as SceneOption[];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When there was only one non-scene option, a SceneOption was created with a value of "". To facilitate more non-scene options, I use their index as the value. It's pretty hacky, but it's simple, easy to work with, and required minimal changes.

const backgroundsLookup = useAppSelector((state) =>
backgroundSelectors.selectEntities(state)
);
Expand All @@ -84,20 +85,9 @@ export const SceneSelect: FC<SceneSelectProps> = ({
}, [scenes]);

useEffect(() => {
setOptions(
([] as SceneOption[]).concat(
optional
? ([
{
value: "",
label: optionalLabel || "None",
},
] as SceneOption[])
: ([] as SceneOption[]),
scenes.map(sceneToSceneOption).sort(sortByLabel)
)
);
}, [scenes, optional, optionalLabel]);
const sceneOptions = scenes.map(sceneToSceneOption).sort(sortByLabel);
setOptions(([] as SceneOption[]).concat(optionalScenes, sceneOptions));
}, [scenes, optional, optionalLabels]);

useEffect(() => {
setCurrentScene(scenes.find((v) => v.id === value));
Expand All @@ -109,6 +99,7 @@ export const SceneSelect: FC<SceneSelectProps> = ({
sceneToSceneOption(currentScene, scenes.indexOf(currentScene))
);
}
else if (optional && optionalScenes.length > 0) setCurrentValue(optionalScenes[+(value || 1)]);
}, [currentScene, scenes]);

const onSelectChange = (newValue: SingleValue<Option>) => {
Expand Down
Loading