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

Added all in-game flowers to designer #102

Merged
merged 2 commits into from
May 23, 2020

Conversation

Zaxyul
Copy link
Contributor

@Zaxyul Zaxyul commented May 21, 2020

Added every flower in-game to the designer (Custom captured and grown assets of the flowers).

For possible compatibility issues I maintained the names of the original flowers that were present in flower.ts and the flower sprite folder but the rest are rewritten in a different format as it was easier during all the work to maintain my own naming scheme.

The filenames are also all different but technically there should be no compatibility issue.

Enabled weeds that were commented out (Wasn't sure why they were commented out but they worked fine so I enabled them).

This took an incredibly long amount of time, I grew/bred/photographed/carefully cut out every single flower in the sprite folder but I think it is a worthy change to now have all the flowers in-game in the designer now for floral planning.

This is also my first pull request in a very long time, I hope I have done it all correctly, if you need anything let me know.

Add every flower in-game to the designer

Added every flower in-game to the designer (Custom captured and grown assets of the flowers).

For possible compatibility issues I maintained the names of the original flower names in flower.ts and the rest are rewritten in a different format.

Enabled weeds that were commented out.
@eugeneration
Copy link
Owner

eugeneration commented May 22, 2020

This looks great - the one comment I have is that I would ideally like the flower names to be a little more consistent. The image file names are fine, but it would best best that all flowers are named 'flowerColor' or 'colorFlower'. I don't really have a preference, but it would be a little easier to follow precedent and do 'flowerColor'.

If you feel 'colorFlower' is best, you will have to add legacy fields that look like this:

// old field
hyacinthWhite: {
  legacy: 'whiteHyacinth',
}
// new field
whiteHyacinth: {
  img: '.../.png',
}

this ensures that maps saved with the old flower names will upgrade to the new naming scheme.

@eugeneration
Copy link
Owner

The reason I'd like to have consistency is so that it's a little easier in the future to do item groupings by flower/color

Normalized the names to be in the flowerColor format instead of a mix of both.

Reverted a few other renamed ones (Example: mumRed to the original chrysanthemumRed) to keep with the original formatting just to keep consistency and remove any need for any legacy references.
@Zaxyul
Copy link
Contributor Author

Zaxyul commented May 23, 2020

Alright I think as you can see above I have now normalized the flower references and reverted some that I had renamed (For one example, chrysanthemumRed to mumRed are now referred to as chrysanthemum's again across the board instead of just mums) just to keep the original naming format in place.

As well as removed pluralizing the flowers where before they were usually the singular wording (rose instead of roses, etc).

I think that should be about everything, let me know if you need any more changes and I will fix that up for you, to be honest I wanted to do this originally but I was very burnt out lol. I believe it should be all correct now!

@eugeneration
Copy link
Owner

Thank you so much!! I think this should be all good!
Thanks for dealing with my nit picks - I'm picky about names because I've been burned by having to change the name later.

@eugeneration
Copy link
Owner

The only thing I'd change (in a later commit) is that the file sizes for the images are a little big (12kb vs the original 3kb)

@eugeneration eugeneration merged commit bc0149a into eugeneration:master May 23, 2020
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.

None yet

2 participants