HomeWildfire Games

New berry bushes by BigTiger
AuditedrP22151

Description

New berry bushes by BigTiger
Reviewed by: @Stan
Comments by: @borg
Thread: https://wildfiregames.com/forum/index.php?/topic/25431-task-trees/

Event Timeline

vladislavbelov raised a concern with this commit.Apr 3 2019, 2:34 AM
vladislavbelov added a subscriber: vladislavbelov.

I got an error on the African Plains map:

ERROR: Could not load mesh 'art/meshes/gaia/berry_bush_02.dae'
ERROR: CObjectEntry::BuildVariation(): Model art/meshes/gaia/berry_bush_02.dae failed to load
/ps/trunk/binaries/data/mods/public/art/actors/props/flora/berry_bush.xml
16

Where is the berry_bush_02.dae in the commit?

/ps/trunk/binaries/data/mods/public/art/meshes/gaia/berry_bush_01.dae
20

I'd prefer to not have personal paths in common files.

This commit now has outstanding concerns.Apr 3 2019, 2:34 AM
Stan marked 2 inline comments as done.Apr 3 2019, 8:03 AM

Thanks for noticing. It's fixed.

/ps/trunk/binaries/data/mods/public/art/actors/props/flora/berry_bush.xml
16

Too many unversionned file in the repository. Missed it. It's fixed now.

/ps/trunk/binaries/data/mods/public/art/meshes/gaia/berry_bush_01.dae
20

Well the exporter doesn't really give us a choice about it. One could make a python scrip that deletes all the useless information in those files, but that's about all we can do.

Stan requested verification of this commit.Apr 3 2019, 8:04 AM
Stan marked 2 inline comments as done.
This commit now requires verification by auditors.Apr 3 2019, 8:04 AM

The model works now without any error, but the berries looks too polygonized. Did you smooth normals?

vladislavbelov accepted this commit.Apr 9 2019, 4:38 PM
vladislavbelov added inline comments.
/ps/trunk/binaries/data/mods/public/art/meshes/gaia/berry_bush_01.dae
71

No new line.

All concerns with this commit have now been addressed.Apr 9 2019, 4:38 PM
Stan marked an inline comment as done.Apr 9 2019, 5:52 PM
Stan added inline comments.
/ps/trunk/binaries/data/mods/public/art/meshes/gaia/berry_bush_01.dae
71

If the parser is dumb enough to get fooled then it should be parsed. Collada is not really that I would call a strong standard

elexis raised a concern with this commit.EditedJun 22 2019, 7:34 PM

Renames to berry_bush_01.png and adds a copy to berry_bush_02.png?

There are two more copies in the same folder with a similar name.

This commit now has outstanding concerns.Jun 22 2019, 7:34 PM
Stan requested verification of this commit.Jun 22 2019, 11:26 PM
Stan marked an inline comment as done.
This commit now requires verification by auditors.Jun 22 2019, 11:26 PM

rP22394 fixes the duplicates from rP22383, but those two files mentioned still look the same?

Stan added a comment.Jun 23 2019, 10:48 AM

The hue is slightly different.

Those are two different areas of the picture.

I will inline them this time:


With imagemagick we can create teh different pixels:

composite ./skins/gaia/berry_bush_01.png ./skins/gaia/berry_bush_02.png -compose difference diff.png

When I zoom in, I see there are slightly different colors, which I don't think anyone can recognize without looking glasses.
If the intention is to create two variants, they should be visually distinct to the player.

elexis accepted this commit.Jun 24 2019, 4:06 AM

If you really want it to be two variations I would suggest to increase the color differences so much that it actually becomes noticeable, even if it doesn't have to be so glaring differences that it becomes considered a different species by the player.
In doubt artistic freedom holds.

It's also not obvious that the previous bush was so bad that it should be deleted. It more seems like a new kind of bush.
(Even if it fits better to this specific map)


It's good to have a large amount of assets to be able to chose from as a map author, so one can picks the ones that fit the map color palette best. So if the actors are really different, I'd recommend to add the new one and allow to chose.

All concerns with this commit have now been addressed.Jun 24 2019, 4:06 AM
elexis added a comment.Jul 2 2019, 8:29 PM

Similar to rP22383, I wonder if the previous bush was deleted, it didn't look so bad that it should be impossible to place it.
I recognize that the bush was reworked, so it started at the previous version, but the new version looks like an entirely new bush (which isn't bad).
The more artwork there is, the more diverse maps can appear, and the more closely map authors can chose assets that fit to the biome and color palette more appropriately, or they can chose to integrate both kinds of bushes for more diversity on the same map.