In all fairness, I’m pretty tired right now, so my thoughts may be off-track!
[edit] Thinking about it, I don’t think that we had consensus on the exact coding style–but I also don’t think that it was directly discussed, and thus that there was also no dissent. However, again, I might be mistaken!
Per SOLID I was referring to this discussion:
After which you’re correct, there wasn’t any dissent. I assumed we wrapped on it. But happy to consider any additional code design ideas at all.
I just awoke, my morning here - so I’m likely to be at this a while. Feel free to rest up! We need our local Thamaturgist healthy! 
Mmm… Perhaps it would have been better that we had discussed it more!
But in all fairness, I’m not sure that I saw SOLID leading to quite this code-style–but conversely and again, since this is a work in progress, I don’t really know what style is is leading to.
Thank you!
It’s still early-ish here, so it’s a while before I sleep, but it is a day of rest for me nevertheless.
We are in a better place now for perspective, I did jump into some Panda sample code to work with it more seriously (previous were cursory reviews).
I really did struggle with Ralph. Honestly - and I’m not a junior programmer, I promise. Now after a little code cleanup I feel a million times more productive. Thinking of this as an experiment, a new user working with a Panda sample of a ‘slight’ bit more complexity than your standard ‘Hello World’. I’m happy to be the guinea pig here =)
After applying some of the very basic concepts of best practices to the sample, it’s like night and day for me in terms of ease of use, understandably.
It’s just something we should consider in the design of slightly-more-complex samples like Ralph (which is my goal with this thread, hopefully to create several of these!!)
If I’m following you correctly, then I do agree in principle! Roaming Ralph was not a great sample, I fear. (Why, oh why did it use that “stop and reset position on ray-hit” method of obstacle collision?! T-T)
Having samples that are clear for new devs seems like a very good idea!
Just to check, are you saying that your reworking of the code is in its final form? (At least in general terms, if not fully bug-free and polished.)
No not close to done yet with the re-vamp. But I’ll definitely be super transparent through the process so it moves in the direction we feel appropriate. (No need for surprises) I can multitask - if folks would like to recommend ideas for the path forward, I’ll listen, for sure.
(If I just wanted to do things my way, I wouldn’t be on this forum, lol!)
Ah, fair enough!
Looking at the code… I’m waaay too tired to review properly now, I’m afraid! So my own comments will perhaps wait until either I get a second wind later, or another day, perhaps tomorrow! ^^;
[edit] One thing that interests me is that it feels almost like a functional style of programming, but implemented using classes.
No problem at all, you’ve already contributed a significant amount!!
For what it’s worth, things are going to change in the code /drastically/ as time goes on, so its OK to wait to see a more indicative structure of any final form.
I could also document/outline all the future plans here - but I figured folks probably just prefer to see code for discussion, too much computer-science theory can make folks a bit weary-eyed, so I’m taking the ‘code as an example’ approach, if that makes sense.
I think that that makes good sense, indeed!
For sure!! To be clear however it’s intended to be more of a ‘declarative’ style of programming. There are subtle differences. Being an architect at my day job I have a big passion for this stuff. If you’re interested at all in more styles of programming (that all really do the same thing in the end, but have huge impact on usability) I had mentioned a wikipedia article (Actually set of articles) up above with really good reading:
And then below that I tried to write a bunch of pseudo code to give an example of how it might work in a 3d game engine sample:
Interesting!
In all fairness, it’s been long enough since I studied this stuff at university that I may be misremembering the relevant labels. ^^;
My own style might have some elements of such an approach, but for the most part I think that I tend to make classes for “things” that have “behaviours”, and have matters like the setup of collision be contained in functions within those classes. But in all fairness I approach it from an intuitive perspective, for the most part, I think.
So I might design Ralph something like this:
(Just showing collision for the sake of brevity and clarity.)
class Game(ShowBase)
def __init__(self):
# All the usual stuff here
self.player = RalphPlayer()
def update(self, task):
dt = globalClock.getDt()
self.player.update(dt)
class RalphPlayer()
def __init__(self):
self.setupCollision()
# Other setup stuff follows
def setupCollision(self):
self.collider = CollisionNode()
# etc. etc.
def update(self, dt):
# Update location, camera, etc.
Some elements might become classes of their own. So if Ralph and his enemies had weapons, I might create a “Weapon” class and possibly sub-classes as appropriate.
The camera might or might not be made into a class: if it’s sufficiently complex, and I feel that I might want to re-use it, then I might; if it’s simple enough, I might confine it to methods of “RalphPlayer”; if it’s very simple, I might not go even that far, just constructing it in “RalphPlayer”'s “__init__” and updating it in the same class’s “update”.
And so on!
For sure, this is great practice! Note in the OO world there is big debate over whether classes should be ‘Nouns’ (things) or ‘Verbs’ (processes) and all perspectives are valid here.
But if you take a step back and look at what you’ve just outlined that you’re doing in your own code - is actually implementing the principles of SOLID! Not to beat the topic as a dead horse (or panda, etc.).
But you’re splitting up components here to appease the ‘S’ (Single use principle). Also addressing ‘loose coupling’ which are parts of the L/I/D.
Normally you’d have more paramters than that (which I’m sure you do in the real world) And this technique satisfies the ‘O’ principle - Open/closed - you can tweak how your classes work via configurable parameters.
As I discovered, you may already be following a standard that others have more formally adapted and named! 
I’m sure! I daresay that I picked up the “noun”/“verb” approach at Uni.
To some degree–but I think that you’ll find more coupling and suchlike than SOLID would seem to imply.
For example, looking back at your description of the “S”, you mention things like “mixing collision, physics, lighting, [and] shader-setup” within a class as being counter to “S”.
But to my mind, to break up relatively small pieces of code–a simple camera-setup, for example–makes little sense, even if the camera arguably has a separate responsibility to the character-movement logic.
Similarly, it makes perfect sense to me for a character-class to handle its own collision-setup; a class just for the collision-objects of a single other class feels superfluous to me.
Anyway, the forum-system has been warning me that I’ve been monopolising the conversation, and I am tired, so I’m going to step back for the day, I think! ^^;
Really? Is this a thing? That’s a shame - you shouldn’t be warned for being such an active contributor!
Yes - agree - remember that guidelines are made to be broken - hence guidelines. Experience helps understand when/where it can be broken.
Even in my ralph rewrite I’ve mixed concepts in the App class, for example, in the init:
self.scene = scene.RalphScene(self)
self.ui = ui.RalphUI()
self.input = input.RalphInput(self)
self.cameraMgr = camera.RalphCameraMgr(self)
self.physics = physics.RalphPhysics(self)
and in the ‘move’:
dt = globalClock.getDt()
self.input.update(dt)
self.physics.update(render)
self.cameraMgr.update()
Surely this is hypocritical right? After touting single-use and then violating it?
But - in this case I feel the ‘Single use’ of the Application main class is to - make the application. Assemble the pieces needed to resolve the requirements in the most clear way possible. Hence that being the single-use principle of this class, I felt OK to mix these concepts together.
In the end its really about the end-user. When they look at the code they should feel like ‘this makes a lot of sense’ and ‘I understand what’s happening’. Overall that should be the guiding principle, the other concepts (like solid) are just means to that end really.
Updates! Lots of refactoring to the code. And a bit of architectural tweaks as well. Also added some friends for Ralph! There is some simple AI in to show his friends walking around.
Also tweaked the controls to use WASD style, much more change coming to the 3rd person camera controls for sure.
See the code here:
Summary of a lot of the new code structure changes in the latest version:
Physics re-vamp. Now Ralph and all his friends share the same physics code. There didn’t seem to be any concept already in Panda to tie together Actors, Animations, AI logic, and collision together, so I made a very thin shell class for this called ‘Entity’. This would serve to demonstrate to the user one way of handling this problem in a real game as well. To make Ralph’s friends this classes was derived for all the logic. I tried to keep all physics self contained in the physics module - because it’s about to get some major changes, and I don’t want to have to refactor all the other code for this work as well.
Lots of generalization in the ‘helper’ code for Ralph. The helpers are no longer tied to the Ralph sample and can be used for other samples as well - and this was the idea to support some features upcoming (Still in progress!)
Things NOT yet done. Need to actually change the physics code to use sliding spheres (as @Thaumaturge has so graciously helped me with). Several other samples are going to get added as well to show how the helpers tie together (Multiplayer with chat UI, 1st person perspectives and other stuff still in the pipe)
One idea in case anyone gets bored, I’d like to try to get stuff from https://quaternius.com/ exported from Blender to Panda. A static and animated mesh just to try things out for the samples. I haven’t had time to get to this yet, but if anyone is able to get it to work I’d love to hear how and what tools you used!!
Thanks!!
-Sal
Not to discourage you at all, but this code is a mess. You’re spawning so many instances of the character that it clips through itself many times. The style is broken up into so many sections, it’s difficult to understand the overall logical plan of the code. This is not good. If you want to code in this style, please do so, but I cannot personally accept it as “canonical Panda3D”, for whatever that is worth.
To go into a little more detail: You are creating separate classes for code that is already so compact that it would be pointless not to just put it in an application definition. There are not absolute and clear standards for things like light management in a generic Panda3D application. In a large team-oriented game engine like Unreal 4, there are no doubt meetings to determine things like the specific Light Attenuation Range for the Standard Light Class, but these standards are meaningless in the context of an unbiased physically based framework. For your own game, do whatever you like with the framework, that’s why it is there. As you have demonstrated, it does not take long to throw some symbolic qualifiers around basic setup code to make things faster for your particular team’s definitions, but enforcing that as some kind of standard model for every developer is way too much. The syntactic complexity of this style is quite higher, it requires assuming what the Light Class for instance does (which will not be documented in the standard manuals), or going through the file hierarchy to read that particular Class itself, which may be complexly linked with your other arbitrarily declared classes.
Thanks for the code review Simulan! And also for downloading and running the sample, I think you may have been the only person to do this thus far, so I’m very appreciative
The plan for the final version of this code is for it to be refactored how the community sees fit! Hence why I’m doing this on a forum, if I didn’t want this to conform to what the community likes I wouldn’t be posting. (In the end, as well, if we communally agree it’s better to package the samples alternatively I’m also up for the idea). I think once we see the final products however we’ll be in a better place to decide. Right now too early to scratch major plans just yet.
My own personal refactoring process is a bit unique, but I promise we can take all agreed-on opinions for the final code structure. It’s a bit of process though… I’ve got a lot to add before we can do the ‘final refactor’.
A few modules I have yet to add, that use a lot of the current code:
Multiplayer - the state of the scene is synced over the socket with a server. Players can join a server and chat with each other (with movement)
Other perspectives: I have 1st person, and top-down perspectives game samples as well in progress
Procedural world: All of the above I also intend to have a sample that shows how the world can be generated procedurally (with interior and exterior/terrain features)
*Alternate media: Some of the above samples should show better use of media (and just nicer media overall) but not such that it grows the media repo too much that it bloats the distro.
Not discouraged at all!!! A few messages up the thread we talked about how this is all code in ‘interim’ state. Things are broken up temporarily in an exaggerated state while we remove all the issues from the original Ralph sample.
Lots of references to lighting in you critique, so I just want to make sure I understand your feedback, so that it’s actionable enough for me to think in terms of code changes.
Here’s /all/ the lighting code I have right now. It was lifted directly from Ralph sample, and put into a method
def setupSimpleLighting(self, ambientLightLevel=.3, directionalLightLevel=1):
ambientLight = AmbientLight("ambientLight")
ambientLight.setColor((ambientLightLevel, ambientLightLevel, ambientLightLevel, 1))
directionalLight = DirectionalLight("directionalLight")
directionalLight.setDirection((-5, -5, -5))
directionalLight.setColor((directionalLightLevel, directionalLightLevel, directionalLightLevel, 1))
directionalLight.setSpecularColor((directionalLightLevel, directionalLightLevel, directionalLightLevel, 1))
render.setLight(render.attachNewNode(ambientLight))
render.setLight(render.attachNewNode(directionalLight))
return [ambientLight, directionalLight]
Are you recommending to remove this from the method and put it back into the main class? If so that’s possible! I do have 3 other samples using the method (hence why I broke it out) - and it’s easier to maintain code when there is a single copy of it.
I could copy/paste the code into each sample I’m working on but that forces me to copy/paste all refactoring and code changes to each codebase - as you can imagine that gets tedious!
Hence why I need to keep code segregated while building these samples. But happy to consider other approaches.
I think the best ‘happy medium’ as that once these samples are working we then re-combine these pieces in a way that fits best for the most ‘canonical’ type of sample. If that makes sense.
Please keep up all the great feedback
Here’s the spawner code:
for i in range(100):
self.addEntity(
RalphsFriend(
app,
media="models/ralph",
anims={"run": "models/ralph-run", "walk": "models/ralph-walk"},
scale=.2, pos= ralphStartPos + (2, 2, 0.5), physicsType="solid"
)
)
Using the same starting point to spawn 100 entities. Totally agree we should tweak this!
To be honest I don’t want this code in the ‘original’ Ralph example. I thought to just keep that as-is, a sample showing Ralph roaming a simple scene.
I think this logic would go into a separate sample whose purpose is to show some AI/NPC interaction features. I had to add this temporarily to test out the new physics code (I refactored Ralph’s physics to be applicable to other entities in the scene, so this needed some testing).
Hope that makes sense!!
Ah, that’s good to read! That was a concern that I was thinking to raise, but it seems that you have it in hand.
As before, I don’t intend to comment on the structure of the code until it’s in a somewhat-final form, I think!
The problem, I think, is that if one or two people are posting a lot, it may discourage others.
This is, funnily enough, my main qualm with what I’ve seen thus far. The more that the reader has to refer to external sources, the less easy it is to follow what’s happening, I fear.