Suggestion to the predefined materials and shaders

Development
21

Yes, you tend to like very small benefits with relatively large costs… and I like the other way around.

22

I don’t see any additional costs here…

23

All code has costs. Every single line has a maintenance cost. The more it does the more it costs.

Your patch manages to take something that should be really simple (since it provides almost no practical benefit) and adds the potential for bugs and additional problems for users that use it. It’s the exact wrong way to balance things.

24

uh I guess we failed to understand each other. I meant a regular constant class with static final strings hard coded.

Also you changed all the occurrences in the engine… So basically with your change if someone changes the properties it will change all occurrence where the material is used… Don’t you see how this could completely go wrong?

25

Hehe… but a user will have to type less when they want… actually, I’m not even convinced that it’s fewer characters to any significance.

The problem this is trying to solve is a different problem completely… and has nothing to do with raw strings in code.

So we definitely shouldn’t be adding places for bugs to creep in.

26

Writing of the same string every time in everywhere… I think it is like something in js style, I don’t agree that is ok for java and that is simple.

27

But you should be using a j3m. And you should be using your own constants.

Besides, when the style leads the rule instead of the rule leading the style then your mindset is completely broken. Whether you type Foo.SOMETHING or “Foo/Something” makes no material (pun intended) difference in this case.

28

I have 2 separated commits, the first commit adds only new constants, the second commit replaces all duplicated strings.

29

I use j3m files because I have a good material editor in jMB and I like to use it. But you can see that you have some users which create materias directly from java code.

30

My point was not the number of commits of your change.
My vision of this was:

public class Materials {
     public static final String UNSHADED = "Common/MatDefs/Misc/Unshaded.j3md";
     public static final String LIGHTING = "Common/MatDefs/Light/Lighting.j3md";
     public static final String PBR = "Common/MatDefs/Light/PBRLighting.j3md";
}

Fixes the original issue raised in this post. Easy enough for people who don’t want to use it to ignore it.
Can be overridden in your project so that you still have MyMaterialConstants.UNSHADED etc…

pspeed edit: I fixed Material to be Materials.
nehon edit: thanks, that’s what I meant :wink:

1 Like
31

so I think you have already written the code here, so that all from me.

32

That was the point, super simple change. I don’t know how you ended up modifying 147 files.
Look, I’m sorry that you wasted your time on this, but it’s always best to keep things as simple as possible, especially for QOL changes like this one.

3 Likes
33

it isn’t a problem for me :slight_smile: I just like to contribute to the engine. I have already had some cancelled PRs and some merged PRs :slight_smile:

34

ok, so no hard feelings. Good.

35

@javasabr Don’t tell me you browse all those files to change them next time when you want to change the similar text in a whole directory use a built-in function in some IDE (I can’t find it on IntelliJ IDEA but it exists on Atom right click on the directory → Search in Directory you will know what to do next.

36

:smiley:

this action named “Find in path…”

37

It doesn’t replace anything it just finds things.

38

Replace in Path :wink:

39

… actually, it’s right under Find in path Replace in Path I finally found it :sweat_smile:

40

hahah yeah i guess he didn’t change all the files manually :stuck_out_tongue: