urho3d / urho3d

Game engine
https://urho3d.github.io
MIT License
4.55k stars 996 forks source link

always provide a default ResourcePrefixPaths #1059

Closed bvanevery closed 8 years ago

bvanevery commented 8 years ago

When trying to migrate the Urho3D Sample Platformer to Urho3D 1.5+ https://github.com/gawag/Urho-Sample-Platformer/issues/12 I've noticed that it must now contend with the new means of handling resource prefix paths. The Setup() function in Urho3D\Source\Samples\Sample.inl has an example:

    // Construct a search path to find the resource prefix with two entries:
    // The first entry is an empty path which will be substituted with program/bin directory -- this entry is for binary when it is still in build tree
    // The second entry is a relative path from the installed program/bin directory to the asset directory -- this entry is for binary when it is in the Urho3D SDK installation location
    if (!engineParameters_.Contains("ResourcePrefixPaths"))
        engineParameters_["ResourcePrefixPaths"] = ";../share/Urho3D/Resources";

This code is borne as a solution to CMake build and installation problems. It's not generally applicable to all possible uses of Urho3D, meaning, people who just use an Urho3D binary and aren't interested in using CMake for their own code. That may sound like a corner case, but my point is the code is more about "contending with CMake" than using Urho3D. Pedagogically, it would be easier for people to write simple code to get going with Urho3D, if they didn't have to worry about such things.

This could be accomplished by establishing a convention, that Urho3D will search the binary directory for resources by default. The programmer could still alter this default behavior however he likes. So, Urho3D would initialize ResourcePrefixPaths to "". This eliminates the need for the following line:

    if (!engineParameters_.Contains("ResourcePrefixPaths"))

as by default it will always contain that. Then provide something like:

engineParameters_["ResourcePrefixPaths"].Append("../share/Urho3D/Resources")

or even more tersely

engineParameters_["ResourcePrefixPaths"] += "../share/Urho3D/Resources"

if you like that sort of style. I didn't look up whether engineParameters_ can do all those sorts of things, I figure you get the idea if it's relevant.

The pedagogical advantage is the user now has to do nothing if their code is simple and they didn't want to worry about installs, or if their install is simple and didn't need a parallel ../bin ../share style directory structure. And if they do need to do something, they have to do less.

I think the pedagogy is important to sustaining the ecology of a 3d engine. Developers get real bored of trying to keep up with engine changes real quick. For instance the Urho3D Sample Platformer developer has pretty much fallen off the radar in recent weeks. He made some changes for some earlier bugs I filed, but then when he saw the 1.5 difficulties he seemed to throw up his hands and vanish. I've been hoping that showing what he needed to do to move the code forwards would keep him from giving up. I myself have had to exercise a certain amount of discipline as I don't actually have his code working yet, just built. I'm having to slug through those very CMake build issues I mentioned above, and I can see why most developers wouldn't relish the complications such build issues bring.

A few years ago, I did a major inventory of the Ogre3D engine ecology. They had listed numerous projects on their website that used their engine. Well, as I worked through each and every last one of them, I found that the vast majority of them were bitrotted and abandoned. I would say only 25% were actually viable as "code I'd currently inflict on people I wanted to keep as friends." I moved all their project descriptions around and put the abandoned projects on a sort of graveyard page. I was trying to give them a bit of a marketing facelift so they'd be putting their best foot forwards, so that code / projects / samples that actually worked and were worth looking at, would be front and center. This taught me a lot about what happens to downstream code, generationally speaking. Changes to an engine kill code. I realize that Urho3D is under rather active development so some of this is unavoidable. But I feel strongly that when it is possible, samples should be easy for developers to write, and easy to migrate between versions. Otherwise it all becomes a huge PITA and in the real world, people give up and move on with their lives.

Oh, and after doing that huge exercise which took me about a month, I concluded there wasn't enough "worth it" with Ogre3D to use it for anything. One particular problem is it saw itself as only a 3D engine, not a game engine. That made the ecology even harder to sustain. For instance, it didn't bless any scripting languages. Bindings to sane things like Lua were third party and bitrotted. One reason I became interested in Urho3D is you did bless a standard inventory of sane stuff.

Someone else did what needed to be done with Ogre3D, and that became the NeoAxis engine. Which cost money. Could easily have been an open source $0 project with a different developer attitude.

friesencr commented 8 years ago

The resource prefix path is a runtime variable. The build doesn't use it afaik. The working directory is the best and most straight forward solution. Anyone shipping a game would use it. If you have multiple projects and want to share a copy of Urho then set an env var. A default value would imply a default build binary output directory which is very different per win/linux/Mac. Changing it would also break other peoples code.

My 2 cents.

bvanevery commented 8 years ago

The build behavior was recently changed to solve the problem posed by #1000 . Build and install search paths are not the same in the samples now.

That change already "breaks other people's code," i.e. Urho3D Sample Platformer. With the rapid pace of Urho3D development, breaking downstream code is inevitable. For instance a number of URHO3D_ prefixes were added to various macros between 1.4 and 1.5, and I wouldn't gainsay that, as it was The Right Thing To Do [TM]. I am proposing this change because I think it would make code easier to migrate, as it breaks.

USP has its own resources. I don't think it is trying to find Urho3D's installed resources, although I haven't rigorously checked that yet.

cadaver commented 8 years ago

If no prefix path parameter is populated, Urho3D will already search only the executable directory for resources. Note that you don't need to use Engine::ParseParameters() for the default command line options or even populate a engine parameters map at all, as the engine should be able to start with sane defaults (if not, it's a bug)

Now looking at it, though I originally thought engine code shouldn't be riddled with install artifacts, I'd rather have the code to insert ";../share/Urho3D/Resources" in one place instead of being repeated in the samples.

Possible place would be Engine::ParseParameters(), since calling that already assumes that the developer will be using the default command line options, and that function is already scanning the system environment for the prefix path anyway.

Or alternatively just initializing the ResourcePrefixPath with ";" in Engine::ParseParameters() if no env var, according to your suggestion.

weitjong commented 8 years ago

I actually don't understand what is the problem being reported in this issue. To me, the engine's default is already being set to the executable directory. See https://github.com/urho3d/Urho3D/blob/master/Source/Urho3D/Engine/Engine.cpp#L215 When nothing is explicitly set then it is defaulted to String::EMPTY which eventually resolves to ProgramDir.

As for the code repetition in the samples and Urho3DPlayer, IMHO it is necessary unless we want to assume that all Urho3D apps including users created one would follow Urho3D project all the way in setting up their installation destination structure. Currently we don't make that assumption as can be seen that the installation destination structure is configured in our Urho3D project's CMakeLists.txt file instead of inside the shared Urho3D-CMake-common.cmake file.

cadaver commented 8 years ago

I agree that it would be a dirty convenience to put the path config to engine code, and best avoided. Certainly it shouldn't be on a code path that is always accessed by engine init (e.g. Engine::Initialize())

Engine::ParseParameters() is somewhere halfway between actual engine code, and convenience code for the samples, as I don't see every Urho3D game wanting to do something like allowing the user to change render path with "-deferred" or "-prepass"

cadaver commented 8 years ago

EDIT: realized that always appending to the resource path creates an unwanted situation where something would be appended to the env.var assigned prefix path, which I don't think should be the intention. So what the samples are doing are indeed correct.

cadaver commented 8 years ago

On further reflection I don't think this can be improved in a way that keeps the engine / install genericity, and any changes now would only serve to confuse more.