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 birch, maple, and willow trees. #13348

Merged
merged 8 commits into from Aug 20, 2015

Conversation

Projects
None yet
7 participants
@Zaweri
Copy link
Contributor

commented Aug 19, 2015

To add harvestables like birch/maple/willow sap/bark. (Adding these in a separate PR most likely.)

Zaweri added some commits Aug 19, 2015

@Zaweri Zaweri changed the title Adds birch and willow trees. Adds birch, maple, and willow trees. Aug 19, 2015

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2015

Ah, glorious. Since one of my projects is yet another tileset update... X3

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2015

We need some standardized tree colors. Currently light green '7' is an apple tree. If you want any non-fruit tree to have light green '7', apple tree needs to have its color changed.

We could apply the same rules from bushes - give apple tree a light green background. This should also apply to all other fruit trees - cherry, plum etc.

Just adding new trees won't make them spawn in game, but mapgen changes can be a separate PR.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2015

I agree with the idea of standardizing fruit trees around the use of a background.

EDIT: Also, I would hope you're adding winter variants and adding a version for birch that's been stripped of bark?

@Zaweri

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2015

@chaosvolt I don't know how to make it turn into that version. I'll get around to all that sometime later; Now I am getting some sleep. Also winter variant would require C++.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2015

Hmm. Get some rest then. I was just gonna say define the terrain entries for the time being, then one can work on implementing them later, or in a separate PR.

Makes fruit trees have colors
Generalized the harvested trees(except for cherry), ripe trees = 7=fruit color, background = ltgreen (except for cherry)
@Zaweri

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2015

Made the fruit trees into different colors now, @Coolthulhu

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

I don't like all those backgrounds. We barely "established" a standard that green background for a plant means "fruit plant" (with fruit bush color change) and now we'd be tearing it down with this new one that colors fruit trees by the color of the fruit and the color of leaves.

I personally would prefer it less colorful: fruit color for the foreground, green/ltgreen for background, with one of those meaning harvested and the other unharvested. That would make few trees look identical, but so far fruit drops are nearly identical. Bushes could be adjusted similarly.

If others prefer the fruit/leaf instead of harvestable/harvested, I'll merge it in the current state, otherwise I'm waiting for the change.

@Zaweri

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2015

@Coolthulhu It is already like that.. fruit color is the 7, background is green except for cherry.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Not just cherry - apple also is wrong.

@Zaweri

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2015

Uhhh... No it is not. Red = apple, cherry is light red background because the tree's leaves are pink.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

All other trees (except cherry and apple) go from something_ltgreen to something_green.

@Zaweri

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2015

Oh, yes, just noticed that after I triple checked, sorry, i have minor dyslexia :/

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Now just the cherry tree. I don't see a reason why would it be an exception. IRL all trees have brown trunks, in game it's green.

EDIT: That is, unless we want all harvested trees to be brown_green

@Zaweri

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2015

Because the background is the color of the leaves. And cherry trees are pink ingame.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

As mention, you might want to add a "t_tree_birch_harvested" terrain to support harvesting birch bark. Whereas it makes sense harvesting from a blackjack oak turning it permanently into a normal oak tree, it makes less sense for birch to turn into an oak tree.

Making the birch tree regrow is optional. For the sake of consistency with blackjack oak, you could opt for the harvested version to be a defunct, non-special tree with no need to add harvest-related properties.

Then all we'd need is to later on code a variant of the blackjack function that can be applied to the standard birch tree.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Because the background is the color of the leaves.

Then the standard is not "ltgreen for harvestable, green for harvested", so I'm waiting either for others to confirm it's OK to have a non-standard coloration or for the change so that it is standard.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Oh and by the way, cherry trees are only red/pink IRL when blooming and that's just like, one season?

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Also pondering how easy it'd be to make the blackjack function modular so that the terrain entry can define what it drops, and what tree it turns into...

@kevingranade

This comment has been minimized.

Copy link
Member

commented Aug 20, 2015

Cherry leaves aren't pink, that's blossoms, and I'm not even sure the cherry trees in New England do that, the ones I'm thinking of are japanese cherry trees. Those are only in bloom for a week or something IIRC. I don't think it's a distinction worth having either, especially if we're trending toward having more tree types.

Zaweri added some commits Aug 20, 2015

@Coolthulhu Coolthulhu self-assigned this Aug 20, 2015

@Chezzo

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

I think they do, they have the Cherry Blossom festival in D.C.! https://en.wikipedia.org/wiki/National_Cherry_Blossom_Festival

Man, kind of makes me want to go through and do cherry_tree_season_spring and all the other spring fruit trees. At least they wouldn't look pickable.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Will be interesting to see this. I'll have to hurry up with my tileset update before these get implemented, given my endless hatred of terrain transparency errors.

I'm assuming that we can puzzle out what the hell to turn harvested birch into once birchbark is actually a thing.

Coolthulhu added a commit that referenced this pull request Aug 20, 2015

Merge pull request #13348 from Zaweri/More-trees
Adds birch, maple, and willow trees.

@Coolthulhu Coolthulhu merged commit e969d28 into CleverRaven:master Aug 20, 2015

1 check was pending

default Started work on pull request.
@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Plums look a little bit too harvested, but otherwise it looks good.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Excellent. So who's planning out the next step? Will they be implementing these AND giving them uses in a later PR, or implementing first and then adding their uses in a third PR?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Next step requires C++. iexamine.cpp, iexamine.h, mapdata.h and mapdata.cpp

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2015

Ah, that's gonna be a hassle to add, I suspect. XP

@illi-kun

This comment has been minimized.

Copy link
Member

commented Aug 21, 2015

Oh and by the way, cherry trees are only red/pink IRL when blooming and that's just like, one season?

Apple trees are white/pink when blooming as well as some other fruit trees.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2015

I'm looking at the code to PR harvesting of birch bark. Will leave the mapgen stuff to others for the time being. Anyone have suggestions for stuff to use birchbark for?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2015

Will leave the mapgen stuff to others for the time being.

Check out how it's done in #13357
It's just 4 lines total.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2015

Hmm. Was looking at the mapgen files, so I guess I could take up the mantle and implement all of the trees. And suggestions for what uses the trees should have?

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Aug 21, 2015

Anyone have suggestions for stuff to use birchbark for?

Birch bark shoes.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2015

Ooh. Partially implementing via adding it as a straw alternative, but dedicated birch bark items would be a nice addition. Also an option, quivers.

Would be tempted to add them as More Survival Tools items, but since I'm already adding mainline uses it might be saner to add such things as mainline.

EDIT: Yeah, going with the sane option of making them in mainline, unless pushing it to mod content is requested.

@illi-kun

This comment has been minimized.

Copy link
Member

commented Aug 22, 2015

Birch bark shoes.

= lapti mentioned in #4938 (they can be produced from bark and not only from bast, see this).

Bark can be used for a lot of things: tents, canoes, hats, boxes....

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2015

Ah right. I did implement birchbark shoes, and made the possibly odd choice of adding birchbark as an alternative for straw in the sandals, hat, and basket recipes.

Should I split that off into completely different items instead? And additionally pondering more uses for willow bark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.