Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeMaterial - PR #7776

Merged
merged 16 commits into from
Dec 17, 2015
Merged

NodeMaterial - PR #7776

merged 16 commits into from
Dec 17, 2015

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Dec 10, 2015

@mrdoob
Copy link
Owner

mrdoob commented Dec 10, 2015

Looking good!

Are you still planning on changing THREE.Node* to THREE.*Node (except THREE.NodeMaterial)?

@sunag
Copy link
Collaborator Author

sunag commented Dec 10, 2015

Yes! I was going to ask you about it. You has a list of names or I can change all except NodeMaterial?

Currently this is the names

<!-- NodeMaterial Base -->
<script src="js/materials/nodes/NodeGL.js"></script>
<script src="js/materials/nodes/NodeBuilder.js"></script>
<script src="js/materials/nodes/NodeRaw.js"></script>
<script src="js/materials/nodes/NodeTemp.js"></script>
<script src="js/materials/nodes/NodeInput.js"></script>
<script src="js/materials/nodes/NodeMaterial.js"></script>
<script src="js/materials/nodes/NodeConst.js"></script>
<script src="js/materials/nodes/NodeFunction.js"></script>
<script src="js/materials/nodes/NodeFunctionCall.js"></script>
<script src="js/materials/nodes/NodeLib.js"></script>

<!-- Accessors -->
<script src="js/materials/nodes/accessors/NodeColors.js"></script>
<script src="js/materials/nodes/accessors/NodeCamera.js"></script>
<script src="js/materials/nodes/accessors/NodeNormal.js"></script>
<script src="js/materials/nodes/accessors/NodePosition.js"></script>
<script src="js/materials/nodes/accessors/NodeReflect.js"></script>
<script src="js/materials/nodes/accessors/NodeUV.js"></script>

<!-- Inputs -->
<script src="js/materials/nodes/inputs/NodeColor.js"></script>
<script src="js/materials/nodes/inputs/NodeFloat.js"></script>
<script src="js/materials/nodes/inputs/NodeInt.js"></script>
<script src="js/materials/nodes/inputs/NodeVector2.js"></script>
<script src="js/materials/nodes/inputs/NodeVector3.js"></script>
<script src="js/materials/nodes/inputs/NodeVector4.js"></script>
<script src="js/materials/nodes/inputs/NodeTexture.js"></script>
<script src="js/materials/nodes/inputs/NodeCubeTexture.js"></script>

<!-- Math -->
<script src="js/materials/nodes/math/NodeMath1.js"></script>
<script src="js/materials/nodes/math/NodeMath2.js"></script>
<script src="js/materials/nodes/math/NodeMath3.js"></script>
<script src="js/materials/nodes/math/NodeOperator.js"></script>

<!-- Utils -->
<script src="js/materials/nodes/utils/NodeJoin.js"></script>
<script src="js/materials/nodes/utils/NodeSwitch.js"></script>
<script src="js/materials/nodes/utils/NodeTime.js"></script>
<script src="js/materials/nodes/utils/NodeRoughnessToBlinnExponent.js"></script>

<!-- Interfaces -->
<script src="js/materials/nodes/interfaces/NodePhong.js"></script>
<script src="js/materials/nodes/interfaces/NodePhongMaterial.js"></script>
<script src="js/materials/nodes/interfaces/NodeStandard.js"></script>
<script src="js/materials/nodes/interfaces/NodeStandardMaterial.js"></script>

<!-- Extras -->
<script src="js/materials/nodes/extras/NodeVelocity.js"></script>

@mrdoob
Copy link
Owner

mrdoob commented Dec 10, 2015

Yeah, I think I would name THREE.*Node all the classes that are meant to be used in the graph.

@sunag
Copy link
Collaborator Author

sunag commented Dec 10, 2015

I will do this with a regular expression /(Node)(\w+)/g to $2$1 . Let's see if it works : )

@sunag
Copy link
Collaborator Author

sunag commented Dec 10, 2015

@mrdoob
Copy link
Owner

mrdoob commented Dec 10, 2015

Hmm, NodeMaterial is a type of Material so I think it should end with Material.

@sunag
Copy link
Collaborator Author

sunag commented Dec 10, 2015

@mrdoob
Copy link
Owner

mrdoob commented Dec 10, 2015

That makes sense to me 👍

@sunag
Copy link
Collaborator Author

sunag commented Dec 10, 2015

Missed MrDoob approves

@mrdoob
Copy link
Owner

mrdoob commented Dec 11, 2015

Hold on a second. PhongNodeMaterial/PhongMaterialNode is a type of Node? Then it should be PhongMaterialNode yes.

@sunag
Copy link
Collaborator Author

sunag commented Dec 11, 2015

PhongNodeMaterial/PhongMaterialNode is a type of Node?

Hmm, PhongNode.js is a type of Node, PhongNodeMaterial is a type of Material.

PhongNode instanceof THREE.GLNode
https://github.com/sunag/three.js/blob/1558f02a70cfaa55973457685df56ee4fdeb41f8/examples/js/materials/nodes/interfaces/PhongNode.js#L7

PhongNodeMaterial instanceof THREE.NodeMaterial
https://github.com/sunag/three.js/blob/1599e1ac2e32bd738615e72b1ba8221df83b2dc9/examples/materials/nodes/interfaces/NodePhongMaterial.js#L9

I am using a proxy to facilitate the use of NodeMaterial in intermediate materials.

@mrdoob
Copy link
Owner

mrdoob commented Dec 11, 2015

Ah ok. PhongNodeMaterial is good then 😁

@mrdoob mrdoob merged commit fb6c118 into mrdoob:dev Dec 17, 2015
@mrdoob
Copy link
Owner

mrdoob commented Dec 17, 2015

Thanks!

@bhouston
Copy link
Contributor

Sweet! Thank you! I'll have a look at this next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants