private class members patch

[tdev]

i dont get why any library coder would create privately declared members instead of protected ones? It completely breaks the idea of derived classes :-/

so please consider this short patch so i can extend the ScriptCompiler for my needs without touching the Ogre code:

Code: Select all

# HG changeset patch
# User tdev
# Date 1313490498 -7200
# Node ID def78069ca3bd878cebd84fb23725d00d645be3d
# Parent  b91d204db4804ce92643437ce2492ac878f97885
changed private members to protected members allowing class derivation

diff -r b91d204db480 -r def78069ca3b OgreMain/include/OgreScriptCompiler.h
--- a/OgreMain/include/OgreScriptCompiler.h	Thu Aug 11 13:58:08 2011 +0300
+++ b/OgreMain/include/OgreScriptCompiler.h	Tue Aug 16 12:28:18 2011 +0200
@@ -114,14 +114,14 @@
 		AtomAbstractNode(AbstractNode *ptr);
 		AbstractNode *clone() const;
 		String getValue() const;
-	private:
+	protected:
 		void parseNumber() const;
 	};
 
 	/** This specific abstract node represents a script object */
 	class _OgreExport ObjectAbstractNode : public AbstractNode
 	{
-	private:
+	protected:
 		map<String,String>::type mEnv;
 	public:
 		String name, cls;
@@ -253,7 +253,7 @@
 		void removeNameExclusion(const String &type);
 		/// Internal method for firing the handleEvent method
 		bool _fireEvent(ScriptCompilerEvent *evt, void *retval);
-	private: // Tree processing
+	protected: // Tree processing
 		AbstractNodeListPtr convertToAST(const ConcreteNodeListPtr &nodes);
 		/// This built-in function processes import nodes
 		void processImports(AbstractNodeListPtr &nodes);
@@ -271,7 +271,7 @@
 		bool isNameExcluded(const String &cls, AbstractNode *parent);
 		/// This function sets up the initial values in word id map
 		void initWordMap();
-	private:
+	protected:
 		// Resource group
 		String mGroup;
 		// The word -> id conversion table
@@ -293,10 +293,10 @@
 
 		// The listener
 		ScriptCompilerListener *mListener;
-	private: // Internal helper classes and processors
+	protected: // Internal helper classes and processors
 		class AbstractTreeBuilder
 		{
-		private:
+		protected:
 			AbstractNodeListPtr mNodes;
 			AbstractNode *mCurrent;
 			ScriptCompiler *mCompiler;
@@ -382,7 +382,7 @@
 	*/
 	class _OgreExport ScriptCompilerManager : public Singleton<ScriptCompilerManager>, public ScriptLoader, public ScriptCompilerAlloc
 	{
-	private:
+	protected:
 		OGRE_AUTO_MUTEX
 
 		// A list of patterns loaded by this compiler manager

UPDATE: seems there are more private's in there:

Code: Select all

OgreMain\include\OgreBillboardSet.h(292):    private:
OgreMain\include\OgreCompositionPass.h(304):    private:
OgreMain\include\OgreCompositionTargetPass.h(148):    private:
OgreMain\include\OgreCompositionTechnique.h(165):    private:
OgreMain\include\OgreCompositor.h(150):    private:
OgreMain\include\OgreCompositorChain.h(215):		private:
OgreMain\include\OgreCompositorInstance.h(289):	private:
OgreMain\include\OgreCompositorManager.h(196):	private:
OgreMain\include\OgreDDSCodec.h(52):    private:
OgreMain\include\OgreDistanceLodStrategy.h(127):    private:
OgreMain\include\OgreException.h(262):	private:
OgreMain\include\OgreFreeImageCodec.h(50):    private:
OgreMain\include\OgreHardwareBufferManager.h(69):    private:
OgreMain\include\OgreInstanceManager.h(85):	private:
OgreMain\include\OgreIteratorWrapper.h(49):	private:
OgreMain\include\OgreMaterialSerializer.h(375):	private:
OgreMain\include\OgreMemoryNedAlloc.h(81):	private:
OgreMain\include\OgreMemoryNedAlloc.h(124):	private:
OgreMain\include\OgreMemoryNedPooling.h(83):	private:
OgreMain\include\OgreMemoryNedPooling.h(126):	private:
OgreMain\include\OgreMemoryStdAlloc.h(87):	private:
OgreMain\include\OgreMemoryStdAlloc.h(143):	private:
OgreMain\include\OgreOptimisedUtil.h(49):    private:
OgreMain\include\OgreParticleScriptCompiler.h(76):	private: // Handlers for compiling script elements
OgreMain\include\OgreParticleScriptCompiler.h(81):	private: // Listener and context data
OgreMain\include\OgrePrefabFactory.h(58):	private:
OgreMain\include\OgreProgressiveMesh.h(87):	private:
OgreMain\include\OgreRenderSystemCapabilities.h(254):	private:
OgreMain\include\OgreSceneManagerEnumerator.h(94):    private:
OgreMain\include\OgreScriptLexer.h(86):	private: // Private utility operations
OgreMain\include\OgreScriptParser.h(54):	private:
OgreMain\include\OgreScriptTranslator.h(274):	private:
OgreMain\include\OgreShadowCameraSetupPlaneOptimal.h(63):	private:
OgreMain\include\OgreShadowCameraSetupPlaneOptimal.h(65):	private:
OgreMain\include\OgreShadowVolumeExtrudeProgram.h(137):    private:
OgreMain\include\OgreSingleton.h(66):	private:
OgreMain\include\OgreSmallVector.h(613):	private:
OgreMain\include\OgreStringInterface.h(163):    private:
OgreMain\include\OgreTagPoint.h(109):	private:
OgreMain\include\WIN32\OgreTimerImp.h(50):    private:
OgreMain\include\OgreUserObjectBindings.h(130):	private:
OgreMain\include\OgreUTFString.h(965):	private:
OgreMain\include\OgreVertexIndexData.h(50):    private:
OgreMain\include\OgreVertexIndexData.h(309):		private:
OgreMain\src\OgreProgressiveMesh.cpp(67):	private:
OgreMain\src\OgreProgressiveMesh.cpp(144):	private:
OgreMain\src\OgreProgressiveMesh.cpp(154):	private:
OgreMain\src\OgreProgressiveMesh.cpp(165):	private:
Components\RTShaderSystem\include\OgreShaderCGProgramWriter.h(135):private:
Components\RTShaderSystem\include\OgreShaderExTextureAtlasSampler.h(350):private:
Components\RTShaderSystem\include\OgreShaderFunction.h(213):private:
Components\RTShaderSystem\include\OgreShaderGenerator.h(920):private:
Components\RTShaderSystem\include\OgreShaderGLSLESProgramProcessor.h(74):private:
Components\RTShaderSystem\include\OgreShaderGLSLESProgramWriter.h(181):private:
Components\RTShaderSystem\include\OgreShaderGLSLProgramProcessor.h(75):private:
Components\RTShaderSystem\include\OgreShaderGLSLProgramWriter.h(135):private:
Components\RTShaderSystem\include\OgreShaderHLSLProgramWriter.h(134):private:
Components\RTShaderSystem\include\OgreShaderMaterialSerializerListener.h(102):private:
Components\RTShaderSystem\include\OgreShaderProgram.h(189):private:
Components\RTShaderSystem\include\OgreShaderProgramManager.h(223):private:
Components\RTShaderSystem\include\OgreShaderProgramSet.h(85):private:
Components\RTShaderSystem\include\OgreShaderRenderState.h(121):private:
Components\RTShaderSystem\include\OgreShaderRenderState.h(206):private:
Components\RTShaderSystem\include\OgreShaderSubRenderState.h(154):private:	
Components\RTShaderSystem\include\OgreShaderSubRenderState.h(213):private:

UPDATE2: ScriptCompilerEvent needs to use private, otherwise it refuses to compile

[CABAListic]

Whoa, whoa, whoa, slow down. We are definitely not going to switch all classes to use protected members instead of private members. Not all classes are meant to be derived from in the first place, and even if they are, there are still legitimate reasons to make a member private and not protected.

So if you want to change any of those, please explain more in detail why you want it changed, i.e. please describe your use case, so that we can make an informed decision.

[tdev]

Whoa, whoa, whoa, slow down. We are definitely not going to switch all classes to use protected members instead of private members. Not all classes are meant to be derived from in the first place, and even if they are, there are still legitimate reasons to make a member private and not protected.

So if you want to change any of those, please explain more in detail why you want it changed, i.e. please describe your use case, so that we can make an informed decision.

well, sorry for going so fast

i want to extend the ScriptCompiler class so i can extract the Nodes information in a custom way from it to represent Materials for my Editor

the idea is to extend the class function with having as little impact on its behavior as possible. I'm open for other ideas though

[Klaim]

Can't you expose the required informations in the class and use it in your class? If it's just accessing data, inheritance is never a good solution.


Also, you might want to read about private, Liskopv principle and other similar principles. Thinking that everybody should be protected but not private is like thinking that everything should be objects. It's abuse.

[tdev]

Can't you expose the required informations in the class and use it in your class? If it's just accessing data, inheritance is never a good solution.


Also, you might want to read about private, Liskopv principle and other similar principles. Thinking that everybody should be protected but not private is like thinking that everything should be objects. It's abuse.

will read up on Liskopv, thanks

also, adding getters for this will bloat the interface with 99% unneeded functions i think :-/

also, still not sure what data i want to use, the abstract tree? the concrete tree? how to get the variables?

so far i got this:

Code: Select all

	DataStreamPtr data = ResourceGroupManager::getSingleton().openResource(origin_filename, origin_group);
	if(data.isNull()) return 1;

	// now the compiler part :)
	Ogre::ScriptCompiler sc;

	String content = data->getAsString();
	String source  = data->getName();

	ScriptLexer lexer;
	ScriptParser parser;
	ConcreteNodeListPtr nodes = parser.parse(lexer.tokenize(content, source));

	sc.compile(nodes, origin_group);

// how to get node data?

UPDATE2: maybe i should write a MaterialTranslator to parse this. But its all hardcoded currently...
its frustrating that there is nowhere high level documentation to be found to this

[dark_sylinc]

Something feels wrong.

a. Are you sure what you need can't be achieved with listeners?
b. ParticleUniverse managed to greatly extend the scripting capabilities without additional efforts. Are you sure you're extending the ScriptCompiler the way it is intended?