JMY1000 / CyclesMineways

Importing script for Mineways into Cycles rendering engine for Blender
GNU General Public License v3.0
7 stars 1 forks source link

Quick fixes needed before release of another version of the script #16

Closed ThomasLynn closed 8 years ago

ThomasLynn commented 8 years ago

It's been a while since we have updated the master branch, so I think we should release a version update.

I think we should bug test, fix any small bugs, and add small features now and then release update 1.2.4.

Any thoughts/suggestions?

JMY1000 commented 8 years ago

We've got enough significant changes that 1.3.0 is probably more descriptive. I'm still testing relatively old changes since I'm so behind you're progress, but yes, that seems like a good idea.

ThomasLynn commented 8 years ago

doing 1.3 would probably be better, yes. I'll test as much as I can, it will be very useful to have different people test this script, as I, for example, like village scenes and glowstone (I'm far too obsessed with villages lit up with glowstone (like... really obsessed with it (I've made about 3 village scenes with glowstone this week))). So I might miss something to do with, say redstone lamps or something.

Masonrf commented 8 years ago

I have made one day and one night render, well, the night one is still going right now, but I'll post it once its done. Both are rather complex but I haven't found any bugs or anything yet. If you can find anything in this, tell me, but I cant seem to find any bugs.

ThomasLynn commented 8 years ago

In the background, it looks like there's black squares, as the image isn't high enough resolution, I can't tell what it is, but it looks like the 2 block high grass bug that I had. I fixed this in 3a8ed78, can you check whether this is the issue (or indeed if there is an issue, as it's hard to see from that far away). There also appears to be black textures on the walkways inside the courtyard (just outside the inner ring)

On another note, Github supports drag and drop image uploading :)

On another note, the Image is overexposed, I'd probably turn down the sun and sky light down a little. If you want, you could decrease the sky light, and keeping the sun light the same, you will get darker shadows, which I think would look nicer. Another thing I'd change is the angle of the sun, ~45 clockwise from where the camera is is normally a good bet. (so the shadows cast to the right) Also, I think you're using water type 1 (???), I'd use water type 2 for this scene, as the water is very flat, which might look weird for water that is out in the open, exposed to wind. Water type 1 has a tendency to look like silk as it's so smooth.

Masonrf commented 8 years ago

Aaah, yes the large flower bug was fixed. I didn't see it there was a new commit before I rendered. I redid the render with the updated script and it removed the black areas.

"On another note, Github supports drag and drop image uploading :)" file was over 10MB :(

Thank you for the lighting advice. I was thinking the same thing about overexposure. I turned down ambient occlusion Water shader type is set to 3. If i remember correctly, I changed some of the values to make it more reflects, as I was actually going for high reflections in the middle area especially.

JMY1000 commented 8 years ago

Personally I'm somewhat of a fan of the clearer water of the old water shader #2. Do you think it might be worth implementing some sort of two variable shading for water, one (roughly) corresponding to color, the other (roughly) corresponding to displacement and texturing?

Masonrf commented 8 years ago

I am not sure what you mean by "color" so this may be pretty much what you were thinking, but what about this:

options for how reflective/transparent the surface is and how smooth/rough the surface is: value 1-4 where 1 is as reflective as possible and 4 is as transparent as possible value 1-4 where 1 is as smooth (no waves) as possible and one is as rough (a lot of waves) as possible

JMY1000 commented 8 years ago

@Masonrf I like that, but I think merging the two into a single "turbidity" variable would be good. By "color" I was referring to how blue/clear the water was.

Masonrf commented 8 years ago

Ok I get what you mean by color now. I like the idea of making the "turbidity" option and a "color" option.

JMY1000 commented 8 years ago

@Masonrf I'll add it to upcomingfeatures.txt

ThomasLynn commented 8 years ago

I think i mentioned a "god" water shader to @JMY1000 a while ago over skype.

I suggest discarding the "water type" way of doing things and just using one shader that reads constants set by the user. This way the user can have conplete control over: "blueness" of the water (minecraft's water is blue, the surface of real water is not) volume absorsion (not in minecraft, but is how real water gets its blueness) type of displacement (like we have now with water types) multiplaction factor for displacement

I think this would also fit nicely with the preposed new GUI, as it could just use sliders.

If we do add these constants, I suggest we don't add any "artifical numbers" to the code (like using between 1-4 instead of 0-1, like it will be behind the scenes)

I would predict the total size of this "god" water shader to be about +40% the size of our current wave shader.

I think this method would allow much more user flexability and will be easier for the user to understand thqn if we decide to add more shaders using the current method.

JMY1000 commented 8 years ago

@ThomasLynn I think I discarded the idea of a god shader earlier due to legibility and modification concerns, but if we redesign all the shading to fit around this god shader instead of the god shader to fit around our current shaders it could work well. I think such a serious redesign should probably wait until we release v1.3.0. If people want to start working on it now, let's create a new branch.

ThomasLynn commented 8 years ago

@JMY1000 I'm not even sure how branches would work with just one file o.o

I think we should just get v1.3.0 out, it appears to be stable. It's got a lot of improvements over the 1.2.3, the last update doesn't even have automatic prefix detection (I feel sorry for the current users :[ )

Unless there's more quick fixes like the big flowers being renamed, I think getting 1.3 out (tonight?) and starting a 1.3.1 brainstorming thread would be a good idea.

JMY1000 commented 8 years ago

@ThomasLynn Agreed. I'm finishing going through the last couple commits right now.

Also creating a new branch would work fine.

ThomasLynn commented 8 years ago

I'm sure it'll work fine, I've just never done it before. <-- git n00b

JMY1000 commented 8 years ago

@ThomasLynn git gud. :P

ThomasLynn commented 8 years ago

ayyyy

Masonrf commented 8 years ago

I like the idea of a god shader thing. I only used the "1-4" example as a way of defining a slider option without actually using a slider, though that would be a good addition to the proposed gui.

"Git gud" << the punsss

Also, question, how well and under what circumstances does automatic prefix detection work?

JMY1000 commented 8 years ago

@Masonrf Reading the code is the best way to understand it.

    if PREFIX == "":
        print("No prefix found, finding best PREFIX")
        names={} # initalises a dictionary
        for img in bpy.data.images: # loops through all images in .blend file
            pos = max( # sets pos to be the max value of the 3 values
                img.name.rfind("-RGBA.png"), # if "-RGBA.png" is in the file name, returns non -1, else returns -1
                img.name.rfind("-RGB.png"), # if "-RGB.png" is in the file name, returns non -1, else returns -1
                img.name.rfind("-Alpha.png")) # if "-Alpha.png" is in the file name, returns non -1, else returns -1
                # all this max statement really does is checks if the string contains any of those strings, if not, it is -1
            print("Checking:",img.name,"(Position: ",pos,"Prefix: ",img.name[:pos]+")")
            if pos!=-1: # if pos==1, it does not contain "-RGBA.png" or "-RGB.png" or "-Alpha.png"
                try:
                    names[img.name[:pos]]+=1 # if a key called the file name in the dictionary exists, increase its value by 1
                except KeyError:
                    names[img.name[:pos]]=1 # this code is only reached if the value could not be increased by one
                    # this happens when the value does not exist (i.e. the key does not exist because this is the first loop)
        print("names: ",names)
        PREFIX = max(names) # finds the name of the key in the dictionary that has the highest value
        # this is how the code determines what the PREFIX should be (majority vote)
    print("Got PREFIX ('"+PREFIX+"')")
ThomasLynn commented 8 years ago

@Masonrf I'll make a pseudocode script (who knew these things might come in handy some day?)

IF PREFIX is not empty THEN
  OUTPUT "No prefix found, finding best PREFIX"
  names <-- BLANK ARRAY
  FOR img =  all images in the .blender file DO
    IF img has "-RGBA,png", "-RGB.png", "-Alpha.png" THEN
      ADD 1 to names[location], WHERE LOCATION IS THE TEXT BEFORE THE FOUND STRING
    ENDIF
  ENDFOR
  OUTPUT ALL names
  PREIX <-- FIND HIGHEST VALUE IN names
ENDIF
OUTPUT "Got PREFIX" + PREFIX
ThomasLynn commented 8 years ago

are we good to push the update?

JMY1000 commented 8 years ago

@ThomasLynn Not quite. Updating the changelog, adding 1.10 support, and working on the night shader.

ThomasLynn commented 8 years ago

@JMY1000 We might clash in our updates. If your update is a big one, you can probably ignore the changes I made in dea3fac. I removed the redundant node that was hanging around the night sky node tree, and made the night sky more dark

JMY1000 commented 8 years ago

@ThomasLynn Except for the night shader we're good to go.

ThomasLynn commented 8 years ago

In its current state, it's fine, I think. I've tried messing around in blender to get better shaped stars, but I can't, so that's probably a job best left for 1.3.1

JMY1000 commented 8 years ago

@ThomasLynn I've submitted some stuff to blender.stackexchange.com, so I might be able to get it resolved soon. I say we wait a bit.

ThomasLynn commented 8 years ago

can you link the post, please?

JMY1000 commented 8 years ago

@ThomasLynn It's only a comment.

ThomasLynn commented 8 years ago

@JMY1000 I think the problem is it's projecting a flat texture onto a sphere. You tried to fix this by just using the camera data, which ignores the fact that the sky is a sphere altogether.

The way I think might be the best is by getting the direction the light ray is coming from, then using that 3d vector the nodes to directly decide on whether to use white or black, without going via a texture.

JMY1000 commented 8 years ago

@ThomasLynn Right, forgot that Blender uses a sphere instead of a skybox. Let me know if you can get that to work.

ThomasLynn commented 8 years ago

I might try tomorrow, or the day after that. I might not be able to do it at all, as it's a logical nightmare using spheres in computers. Gotta get some sleep now, drive theory test tomorrow.

EDIT: passed it!

ThomasLynn commented 8 years ago

Are we ready to push the update?

JMY1000 commented 8 years ago

@ThomasLynn Still working on the night shader, but we should probably just push it as is for now. I've added it to the top of upcomingfeatures.txt

JMY1000 commented 8 years ago

Pull request submitted, if no one has any objections I'll merge and close this issue.

ThomasLynn commented 8 years ago

Did a once over, looks good.