Page MenuHome

UPBGE
AbandonedPublic

Authored by Porteries Tristan (panzergame) on Jan 9 2017, 6:22 PM.

Details

Summary

This patch is the differences between the blender master and UPBGE master, it's a quite big patch. The interest of this is not to review line per line the patch, checking syntax ect…, but to give you a general idea of which parts the UPBGE is trying to modify. We need your feedback to allow us refactoring some of our modifications to intent to be accepted in blender 2.8.
Some parts in the patch are to ignore because they are just here to help UPBGE only development (e.g .gitmodules, versioning, splashscreen). Feel free to ask any questions on the part modified, but please be generalist

All the documentation about feature and bug fixes are in doc.upbge.org, an example of the python api documentation can be found at pythonapi.upbge.org.
For deeper questions, jumps into IRC at #upbgecoders.

Thanks in advance.

Diff Detail

Event Timeline

Update diff without source/gameengine modifications.

Here is a very short list of the modification per non game engine directories:

  • doc/:
    • Add of python api doc (about doc, naming, tooltips etc... we'd need someone who speaks well english)
  • intern/:
    • ghost:
      • Xinerama (I don't know if this has to be reviewed. The usage is limited to gameengine?)
    • string:
      • std;;string constructor
  • releases/:
    • Ui modifications
  • source/:
    • blender/:
    • blenkernel/:
      • python component (we'd need Moguri and another UI dev for that I guess?)
    • makesrna/:
      • new object/material/texture properties
    • editors/:
      • debug draw
      • logic editor modifications
    • gpu/:
      • GI, constant values, uniform pointer…
      • render buffer
    • makesdna/:
        • new object/material/texture properties
        • python component
      • UI python
    • python/:
      • bpy modules dict fixes

For the part about UI @Bastien Montagne (mont29) could be one of the reviewer, about the documentation advices about English spelling are welcome (none of the more active developers are English native), and finally the part gpu is modifying the shaders, material and off screen management, here the help of @Mike Erwin (merwin), @Dalai Felinto (dfelinto) and @hypersoniac would be great.

An other thread is the C++11 support, some of the game engine sources are using C++11 features and they are difficult to back to C++98/03. Will the C++11 support be acceptable for blender 2.8 ?

Thanks everyone in advance for review and comments.

C++11 will be mandatory for Blender2.8 (@Sergey Sharybin (sergey) also need it for new depsgraph afaik), so that should not be an issue, provided you do not use 'forbidden' features of this new version (though this is not yet fully decided, again @Sergey Sharybin (sergey) shall know more here).

Also, did very quick first skim in this code, and there is one thing that hurts my eyes currently: that PythonComponent!

  1. Why does this exists in BKE? Afaik, we do not want anything python-related in this part of the code? Why do they need to be in Blender code itself at all, actually, if they are only UPBGE stuff?
  2. Afaics, this is merely re-inventing ID properties, any reason not to use them instead?
  3. The DNA struct for them is not acceptable for me, storing a float, an array of floats, a string, a bool, etc., just to mimic a multi-type storage? come on!

Maybe I’m missing some obvious stuff here, but really cannot understand neither the need nor the implementation of this PythonComponent stuff, maybe some more explanation, background and reasoning about this is needed in the first place?

Also, why do you need upbge versionning, separated from 'normal' Blender versionning? This sounds fishy to me as well…

source/blender/blenloader/intern/writefile.c
1423–1446

Please get rid of that kind of differences! Not only are they generating useless noise (a lot!), but this is breaking our codestyle (same below of course).

Right, C++11 and C99 are finally available on all the platforms/compilers we target. I use C++11 in other projects and am a big fan.

I'll do a closer review soon...

source/blender/blenloader/intern/writefile.c
1423–1446

+1 to what @Bastien Montagne (mont29) said. So we can focus on the real changes.

@Bastien Montagne (mont29) : The UPBGE versioning can be forgotten of the review.
About PythonComponent, the properties are create when we create the python component so when we load the python component class. To find these values we need to acces the python component class args attribute, see https://pythonapi.upbge.org/bge.types.KX_PythonComponent.html for more details.

I agree that BKE isn't the correct place and that my choice was wrong, will be source/python a better place to call a function finding the component properties ?

For the property struct, you advice to use a separated type with at first value the type identifier ? Or reuse an existent type allowed in BKE ?

.gitmodules
1–16 ↗(On Diff #8142)

Un-related changes.

README.md
1–14 ↗(On Diff #8142)

un related

.gitmodules
1–16 ↗(On Diff #8142)

It's advised to ignore these parts from my original differential summary.

This is so good to see this patch prototype begun to be reviewed :) hehe. Thanks very much for your review and comments.

@Porteries Tristan (panzergame): as it seems a "real" review has begun, maybe we should take into account the first comments and propose a cleaned version of the patch without versioning (or with a commented (I mean commented with /**/) versioning integrated to versioning_270.c, remove parts that concern only upbge (splashscreen, git...), fix the parts where we adeed whitespaces or change blender codestyle, do something for python_components...

And come back with something cleaner and easier to review the more quickly possible (you see why we'd need Engligh language native people to help us to fix doc and tooltips :P).

@Porteries Tristan (panzergame): About C++11, you begun to use it very recently, no? I was wondering if in a first time we could propose a patch without the C++11 parts (without mesh batching :/), and wait for official C++11 and MSVC 2015 support to propose another merge with BF. Like that we would already have our first year of work integrated into blender, and already have a foot in Blender official -> advantage: The faster we merge into Blender official, the earlier all Blender devs see the changes we did in Blender sources (So they can begin to be familiar with the few changes we did and eventually use it if they are interested).

(And yes, about python components panzergame question, this would be great if you could advice us about the places where you'd have wrote the code @Bastien Montagne (mont29) . Thanks very much!).

youle, hobbyist beginner upbge coder

@Ulysse Martin (youle) it sounds like the C++11 parts can be kept as long as they follow certain rules (we'll need @Sergey Sharybin (sergey) to clarify what those rules are). I am assuming this merge will ultimately target 2.8 since a feature set this large does not seem appropriate for 2.7x.

Regarding PythonComponents, if you really have to put it in main code, it would be in source/blender/python, yes. But why can’t it stay in bge code?

Also, briefly talked with @Porteries Tristan (panzergame) yesterday, imho you could use IDProperties (see DNA_ID.h and BKE_idprop.h) for that purpose.

@Mitchell Stokes (moguri) : About C++11 we have some rules:

  • avoid as much as possible auto
  • prefer range loop iteration
  • use using keyword

@Bastien Montagne (mont29) : About the python usage, we can't move these part in the gameengine directory because the python component is created from the UI outside the game.

Update without unrelated modifications (per file).

@Dalai Felinto (dfelinto) Dome code has been removed from UPBGE. Can you comment on if this is a problem?

@Dalai Felinto (dfelinto) Dome code has been removed from UPBGE. Can you comment on if this is a problem?

Only to remark. The code was removed because it was a bit hacky and we were doing a rasterizer clean up and update. If you consider that it is necessary to allow the merge we can see how to update and to integrate it in current gameengine code.

I spent more time these days on the usage of IDProperty for python components but i encountered some issues:

  • There is a bool property ? A property displayed in UI as a simple check box.
  • How to make a property owning the selected value in a enum ?
source/blender/blenlib/BLI_utildefines.h
47

ehhh, i will recheck that…