Page MenuHomeWildfire Games

Template organization: flora and fauna
Needs RevisionPublic

Authored by Nescio on Nov 3 2017, 9:34 PM.

Details

Reviewers
Stan
Summary

This patch:

  • merges fish, fish_tilapia, and fish_tuna (three identical files)
  • changes the parent of the muskox from domestic into hunt_skittish (muskoxen have never been domesticated)
  • renames rhino to rhinoceros (cf. crocodile)
  • moves gaia/fauna_* to gaia/fauna/*
  • moves flora_tree_medit_fan_palm.xml to flora/tree_fan_palm_mediterranean.xml
  • moves flora_tree_palm_tropic.xml to flora/tree_tropical_tall.xml
  • moves flora_tree_senegal_date_palm.xml to flora/tree_date_palm_senegal.xml
  • moves gaia/flora_* to gaia/flora/*
  • corrects all corral production queues

To correct the maps/ files, run this script in your 0ad/ repository:

See also discussions at D989 and #4770

Test Plan

Apply the sed script, check if everything works, and nothing is overlooked.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 3599
Build 6247: Vulcan BuildJenkins
Build 6246: arc lint + arc unit

Event Timeline

Nescio created this revision.Nov 3 2017, 9:34 PM
Owners added a subscriber: Restricted Owners Package.Nov 3 2017, 9:34 PM
Nescio added a comment.Nov 3 2017, 9:43 PM

NB It appears the sed script can't read map files which contain spaces (" ") in their name. Any suggestions on how to solve this?

Vulcan added a subscriber: Vulcan.Nov 3 2017, 10:23 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
  const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
           ^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36:   required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ s
In D1009#39578, @Nescio wrote:

NB It appears the sed script can't read map files which contain spaces (" ") in their name. Any suggestions on how to solve this?

Will need a slight change to how sed is invoked, as whitespace is a parameter seperator (one can change that by setting the IFS variable, but for this case it would be slightly more complicated, or rather somewhat uncommon).
We'll first invoke find, make it use \0 as the separator, then use xargs to split that and pass it to sed.

So roughly find path/to/things -print0 | xargs -0 sed -i 's/a/b/g;s/c/d/g'. Or if you put the sed commands in a file as explained below find path/to/things -print0 | xargs -0 sed -f path/to/script.

And to answer the part above, sed is not involved in dealing with splitting parameters, that's what the shell does (and the default is splitting on whitespace), if you want to pass one parameter with whitespace you have to escape it (prefix with`\) or quote it (enclose in "" or ''`), or use the tools in such a way that they handle this for you.

Also your script is more complicated than needed. You can use -name '*.xml' -o -name '*.js' instead of two lines for the same thing. Then you can also use 's/a/b/g;s/c/d/g' to combine a few things. Or put all those s/a/b/g commands on one line each and passing that to sed, which would make it a sed script, instead of a generic script. Instead of find */ you could also use find . which also checks files in the current directory, or specify the full path from eg the 0ad root directory (so find binaries/data/mods/public).

I guess one could also do the moving in a script and just propose that thing, not sure if that'd make things easier or not, then again I'm not doing the review.

Nescio edited the summary of this revision. (Show Details)Nov 4 2017, 1:44 PM

The following commands should work:

find binaries/data/mods/public/maps/ \( -name '*.js' -o -name '*.xml' \) -print0 | xargs -0 sed -i \
    -e 's,gaia/fauna_fish,gaia/fauna/fish,g' \
    -e 's,gaia/fauna_fish_tilapia,gaia/fauna/fish,g' \
    -e 's,gaia/fauna_fish_tuna,gaia/fauna/fish,g' \
    -e 's,gaia/fauna_rhino,gaia/fauna/rhinoceros,g' \
    -e 's,gaia/fauna_,gaia/fauna/,g' \
    -e 's,gaia/flora_tree_medit_fan_palm,gaia/flora/tree_fan_palm_mediterranean,g' \
    -e 's,gaia/flora_tree_palm_tropic,gaia/flora/tree_palm_tropical_tall,g' \
    -e 's,gaia/flora_tree_senegal_date_palm,gaia/flora/tree_date_palm_senegal,g' \
    -e 's,gaia/flora_,gaia/flora/,g'

grep -Ri "gaia/fauna_" binaries/data/mods/public/maps/
grep -Ri "gaia/flora_" binaries/data/mods/public/maps/

What I immediately noticed is that there are some *.json files containing the search strings as well. Not sure if they should be changed as well.

Stan requested changes to this revision.Dec 29 2018, 1:15 PM
Stan added a subscriber: Stan.

Did you run the above scripts ? :) Also, this will break all mods.

This revision now requires changes to proceed.Dec 29 2018, 1:15 PM
elexis added a subscriber: elexis.Dec 29 2018, 2:08 PM

merges fish, fish_tilapia, and fish_tuna (three identical files)

That should be a bug, many maps have fish that makes no sense there (thuna is big, doesnt occur in rivers and not in all geographic areas), there should be 3 or more distinct fishes with distinct names that fit to the maps.
There might be a ticket for that unless me and the other witnesses skipped writing it.

Wanted to say somehting about splitting this into smaller things; For example the goat + civ file modifications looks like it would be better off separately; But then the affected lines files would have to changed twice, so it seems okay as is.

(Fun with merge conflicts in every file with #5366 but I guess I already knew that when I started the branch)

Did you run the above scripts ?

Yes, I did, and they worked. That was over a year ago, though, I guess before the introduction of Kushite content, so I will have to check again if.

Also, this will break all mods.

Only those that have maps; other mods are unaffected.

That should be a bug, many maps have fish that makes no sense there (thuna is big, doesnt occur in rivers and not in all geographic areas), there should be 3 or more distinct fishes with distinct names that fit to the maps.

Last time I looked at it merging the fish seemed the most sensible thing to do. More types of fish could be added later, of course.

Stan added a comment.Dec 30 2018, 4:16 PM

I have a Tuna fish made by a student. There is also a version made by micket though sadly Enrique's texture has not been bundled with. Need some animations of course. I haven't still decided whether it's better to have incomplete animals and receive bug reports or no animal at all.

moves gaia/fauna_* to gaia/fauna/*
moves gaia/flora_* to gaia/flora/*

If I wanted to ask question, I would ask: does you need to put those things in separate folders? What issue does it solve?

changes the parent of the muskox from domestic into hunt_skittish (muskoxen have never been domesticated)

(Until nowadays. But well that's not the debate.)
If I agree about what you mean, you should before check what it means in game meaning (domestic "code" meaning is not perhaps domestic "real life" meaning). Here I guess it's fine (but it's not my job).
(To not be misinterpreted, I really don't say the change is bad!).

(Despite that if you are interested in that, I think there was a confusion with the planned Auroch (see https://trac.wildfiregames.com/wiki/List%3A_Entities%3A_Nature%3A_Fauna%3A_Herd and #1970) and I am not sure even that one fit in the "0ad" timeframe.)