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

Add PAL palette format import #315

Conversation

matthewpaul-us
Copy link
Contributor

@matthewpaul-us matthewpaul-us commented Aug 25, 2020

Added a PAL importer to fix #307. I used a palette from Lowspec to test: Isa's Limited Ramps

Before palette import
Before I imported the palette, you can see that it doesn't exist.
image

Adding the accepted file formats
image

Post-import Palette
The palette takes the file name as the palette name. Note there are 28 colors which match the number of colors in the palette.
image

Let me know if I need to change anything.

Copy link
Member

@OverloadedOrama OverloadedOrama left a comment

Choose a reason for hiding this comment

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

I think it looks good! Just a minor detail, I think it's best to change the PaletteImportFileDialog's filters in its own scene (PaletteImportFileDialog.tscn), instead of PalettePanelContainer.tscn.

It would also be good to make it possible to import PAL palette files from the Web version too, by making changes to the HTML5FileExchange.gd script. It's not a big issue though, I can do that myself if you want.

@matthewpaul-us
Copy link
Contributor Author

Ah! I missed it on the file dialog itself. I'll make that change definitely. Thanks for the catch!

I'll also look into the HTML5FileExchange to see the difficulty. If it's not too bad, I'll include it.

@matthewpaul-us
Copy link
Contributor Author

I moved the selection filter into the file dialog.

I attempted the HTML change, but I wasn't sure how to test it. I exported it to HTML5 and then ran it using a python web server. When I went to import the palette, it threw a javascript error. I think it's because I am using the same palette_data as the gpl, when I need the text back from the javascript. I'll keep looking at it.

image

@matthewpaul-us
Copy link
Contributor Author

I was able to get back to this and saw my issue. I had forgotten the s on ends_with which caused it to fail. After fixing this and an issue with javascript comparison not liking the not-equals, it works in the web as well.

Copy link
Member

@OverloadedOrama OverloadedOrama left a comment

Choose a reason for hiding this comment

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

Okay I think everything is good now. Thank you!

@OverloadedOrama OverloadedOrama merged commit 348d758 into Orama-Interactive:master Aug 26, 2020
@matthewpaul-us matthewpaul-us deleted the add-pal-palette-importer branch August 26, 2020 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.pal file type for color palettes.
2 participants