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

Adding particle blend, glow and animation #4705

Merged
merged 1 commit into from Nov 14, 2016

Conversation

@Foghrye4
Copy link
Contributor

Foghrye4 commented Oct 30, 2016

This pull could add particle blend, glow and animation to particle and particle spawners.
Solve #4232
How it looks: https://youtu.be/9y-izjlxPmE
Test code: http://pastebin.com/URY3QitQ (outdated)
Test texture: http://imgur.com/c5sidRL

@sfan5

This comment has been minimized.

Copy link
Member

sfan5 commented Oct 30, 2016

I really dislike exposing Irrlicht constants directly to Lua. Otherwise this seems useful concept-wise.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Oct 30, 2016

@sfan5 on my opinion - its a best option to not limit modders for several pre-defined blend types, but give them opportunity to experiment with them and support those, who already knows how OpenGL and Irrlicht works.


Edit: found an incorrect comment, fixed.

@sfan5 I would like to see your option for defining particle material blend. May be we will reach a compromise.

@sfan5

This comment has been minimized.

Copy link
Member

sfan5 commented Oct 30, 2016

@Foghrye4 the problem with this is that (should we ever) when we switch from Irrlicht to a different engine we have to support all of the behaviours of these possibly Irrlicht-specific effects.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Oct 30, 2016

@sfan5 I don't get it. Do you plan to switch to another engine in a close time? As soon as I understand, to switch to another engine we need to rewrite almost entire client part of a Minetest code, isn't so?

If you are planning to switch to another engine in 2018 I will rewrite Irrlicht-related part of code to match new engine in December of 2017.
If you are planning to switch to a new engine in a few months, I guess there is no options and this pull should be closed.

Ah, what you trying to tell me, that you want to preserve back compatibility of old games with another engine, yes?

@obneq

This comment has been minimized.

Copy link

obneq commented Oct 30, 2016

ugh. the old "let's switch engines" stuff. imho it is a non issue, because none of mts problems are caused by the choice of engine and no problem will be solved with a switch. mt would just be abusing another engine badly. it would be better to clearly and cleanly use what irrlicht has to offer in a proper way and probably also a more sane first step towards engine independence, seeing as most of those share some similarities whereas mt is still very confused about what to do (half-ass) itself and what to use whatever engine for.

src/script/lua_api/l_particles.cpp Outdated
if (first_frame >= vertical_frame_num) {
std::ostringstream error_text;
error_text << "first_frame should be lower, than "
<< "vertical_frame_num. "

This comment has been minimized.

Copy link
@est31

est31 Oct 30, 2016

Contributor

only one tab more

src/script/lua_api/l_particles.cpp Outdated
int ModApiParticles::l_add_particlespawner(lua_State *L)
{
MAP_LOCK_REQUIRED;

// Get parameters
u16 amount = 1;
u16 vertical_frame_num = 1;
u16 min_first_frame, max_first_frame;
min_first_frame= max_first_frame= 0;

This comment has been minimized.

Copy link
@est31

est31 Oct 30, 2016

Contributor

use only tabs for indentation

@est31
Copy link
Contributor

est31 left a comment

Otherwise looks good.

src/server.cpp Outdated
u16 attached_id, bool vertical, const std::string &texture, u32 id,
u32 material_type_param, u16 vertical_frame_num,
u16 min_first_frame, u16 max_first_frame, float frame_length,
bool loop_animation, video::SColor &glow)

This comment has been minimized.

Copy link
@est31

est31 Oct 30, 2016

Contributor

make this glow param const, everywhere where its passed.

src/script/lua_api/l_particles.cpp Outdated
collisiondetection, collision_removal, vertical, texture);
getServer(L)->spawnParticle(playername, pos, vel, acc, expirationtime,
size, collisiondetection, collision_removal, vertical,
texture, material_type_param, vertical_frame_num,

This comment has been minimized.

Copy link
@est31

est31 Oct 30, 2016

Contributor

only one indent more, not two

src/script/lua_api/l_particles.cpp Outdated
if (max_first_frame >= vertical_frame_num) {
std::ostringstream error_text;
error_text << "max_first_frame should be lower, than "
<< "vertical_frame_num. "

This comment has been minimized.

Copy link
@est31

est31 Oct 30, 2016

Contributor

here too

builtin/common/misc_helpers.lua Outdated
@@ -238,6 +238,47 @@ function math.sign(x, tolerance)
end

--------------------------------------------------------------------------------
function math.intpow(x, y)

This comment has been minimized.

Copy link
@est31

est31 Oct 30, 2016

Contributor

I'd prefer if you didn't add this function and instead wrote out the values manually. Its not really implemented efficiently (doing y many multiplications), an efficient implementation would look like this: https://github.com/rust-num/num/blob/c8ed8ff87be3882854aa6f4d780bd34e3613545d/traits/src/pow.rs#L25

It has logarithmic many multiplications while your implementation has linear many.

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Oct 30, 2016

@obneq wrong, in fact there are quite some bugs caused by upstream. And no, I don't suggest we should change engines, but that doesn't mean we never should.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Oct 30, 2016

@est31 requested changes are done.

@est31

est31 approved these changes Oct 30, 2016

@sfan5
Copy link
Member

sfan5 left a comment

wat

doc/lua_api.txt Outdated
* left by x bits for brightening. Contain:
* modulate_1x = 1 -- no bite shift
* modulate_2x = 2 -- 1 bites shift
* modulate_4x = 4 -- 2 bites shift

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

bytes

doc/lua_api.txt Outdated
* vertex_color = 1 -- use vertex color alpha.
* texture = 2 -- use texture alpha.
* 'minetest.pack_texture_blend_func(srcFact, dstFact, modulate, alphaSource)': return integer
* pack texture blend funcion variable.

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

this isn't very descriptive

doc/lua_api.txt Outdated
@@ -4094,6 +4120,24 @@ The Biome API is still in an experimental phase and subject to change.
-- ^ Uses texture (string)
playername = "singleplayer"
-- ^ optional, if specified spawns particle only on the player's client
material_type_param = minetest.pack_texture_blend_func(
minetest.ebf.src_alpha, minetest.ebf.one,minetest.emfn.modulate_1x,
minetest.eas.texture + minetest.eas.vertex_color),

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

omit this big example?

This comment has been minimized.

Copy link
@Foghrye4

Foghrye4 Oct 31, 2016

Author Contributor

I prefer not. Examples are important for understanding of what those variables actually doing.

doc/lua_api.txt Outdated
loop_animation = true,
-- ^ optional, specify if animation should start from beginning after last frame.
glow = "#AAAF"
-- ^ optional, specify particle opacity and self-luminescence in darkness.

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

is this a ColorString?

This comment has been minimized.

Copy link
@Foghrye4

Foghrye4 Oct 31, 2016

Author Contributor

@sfan5 ColorString or indexed color. Both are acceptable by parsing function. Notes added in Lua API doc.

src/particles.cpp Outdated
u16 first_frame,
float frame_length,
bool loop_animation,
const video::SColor glow

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

const not needed (did you mean to pass by reference?)

This comment has been minimized.

Copy link
@Foghrye4

Foghrye4 Oct 31, 2016

Author Contributor

@sfan5 est31 ask me to add const on passing this variable at server side and I mindlessly add const keyword at client side as well. My bad.
Why we all have numbers? I wonder if there is Foghrye3 somewhere in parallel universe...

src/script/lua_api/l_particles.cpp Outdated

std::string texture = "";
std::string playername = "";

u32 material_type_param = 8464;

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

again...

src/script/lua_api/l_particles.cpp Outdated
lua_getfield(L, 1, "glow");
if (!lua_isnil(L, -1)) {
read_color(L, -1, &glow);
}

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

brace removal possible

src/script/lua_api/l_particles.cpp Outdated
ServerActiveObject *attached = NULL;
std::string texture = "";
std::string playername = "";
u32 material_type_param = 8464;

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

again...

src/script/lua_api/l_particles.cpp Outdated
lua_getfield(L, 1, "glow");
if (!lua_isnil(L, -1)) {
read_color(L, -1, &glow);
}

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

brace removal possible

src/script/lua_api/l_particles.cpp Outdated
min_first_frame, max_first_frame,
frame_length,
loop_animation,
glow);

This comment has been minimized.

Copy link
@sfan5

sfan5 Oct 30, 2016

Member

this should be kept as two indents

@obneq

This comment has been minimized.

Copy link

obneq commented Oct 31, 2016

@est31 sorry but your reply sounds to me as if you stopped reading my post/rant after the first sentence, as we basically come to the exact same conclusion. regarding upstream bugs, other engines have bugs as well i guess

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Oct 31, 2016

@obneq when I wrote that answer, I already had read your post completely.

I don't want to continue arguing about this in this thread, its offtopic.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Oct 31, 2016

@sfan5 requested changes are done.

@sofar

This comment has been minimized.

Copy link
Member

sofar commented Oct 31, 2016

I prefer seeing animated textures implemented in the same way that node textures are animated. This is an existing API and works well:

https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L3648

then blend and glow could be simple additional parameters. It would also make the documentation a lot simpler.

All this OpenGL stuff is cool and ""WOW FREEDOM"", but it isn't going to appeal to the average mod writer, it may even deter them. Most likely, everyone will use the exact thing that's in the example and copy that from one another over and over.

Watching the video, I don't see any glow or blending? Or is the blending the same as texture alpha handling? If so, why would we not enable this by default?

The packing of flags makes for a hugely complex API. If this is really needed and can't be simplified to the core parts of what is needed, I'd really like to see all the flags defined as actual constants (e.g. pass them as arrays of strings "modulate_2x" instead of defined constants, but really, I'd like all of this magic to go away.

Don't get me wrong, I'm delighted that this works, but the interface really looks like a liability.

Maybe to summarize, can you answer this question:

Would any mod writer ever do more than "vary glow factor" and "vary blend factor" (or some other limited range of factors) in a way that requires all those bits to be exposed?

Also, why wouldn't we just expose the bitfield directly (e.g. just pass an int from lua) and document the bits separately? Packing them every time a particle generator is made also seems wasteful, although that's likely just marginal, but it's still a ton of table lookups).

Related: #4215

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Oct 31, 2016

@sofar

It would also make the documentation a lot simpler.

I need to add glow and blend to tiles first.

Packing them every time a particle generator is made also seems wasteful,

Indeed. It up to modder to pre-calculate and store those values somewhere.

Watching the video, I don't see any glow or blending?

Texture used in example have no alpha channel. Also in a given in example video cave those particles will be as barely visible as a walls, because they would be rendered as dark as a walls.

Would any mod writer ever do more than "vary glow factor"

Glow is not a factor, it is just a colour added to a light levels defined from environment.

"vary blend factor"

Using it you not just can turn brightness of a picture in a transparency. For example if you made a texture with some kind of shape with colour 662211h it will appear dark red with a single particle, then yellow with a several of them and white if player looking thru a lot of them. With this you could made a nice-looking fire.
example

All this OpenGL stuff is cool and ""WOW FREEDOM"", but it isn't going to appeal to the average
mod writer, it may even deter them.

I will answer with one of my favourite quotes:

The whole principle is wrong; it's like demanding that grown men live on skim milk because the baby can't eat steak.

src/network/clientpackethandler.cpp Outdated
event.spawn_particle.vertical = vertical;
event.spawn_particle.texture = new std::string(texture);

event.type = CE_SPAWN_PARTICLE;

This comment has been minimized.

Copy link
@SmallJoker

SmallJoker Oct 31, 2016

Member

After the first letter, always indent with spaces, otherwise it will look ugly with different tab sizes. (Same for your Lua scripts)

@sofar

This comment has been minimized.

Copy link
Member

sofar commented Oct 31, 2016

The whole principle is wrong; it's like demanding that grown men live on skim milk because the baby can't eat steak.

A witty quote? I prefer a technical answer.

You didn't comment on the existing tiledef API at all.

I can live with new bit flags, even the hardcoded tables, but the tiledef table format should really be re-used, instead of creating a new tiledef API that's essentially the exact same thing, but unfolded in the larger table. That's just bad form, and no excuse to hide behind a new API.

* `{type="vertical_frames", aspect_w=16, aspect_h=16, length=3.0}`

Sure, you may have to frob loop in there (node animations are always looped) but it sure would be nice to have a particledef that looks like this instead:

minetest.add_particle({
            pos = vector.add(vector.new(0,1,0), vector.add(player:getpos(),player:get_look_dir())),
            velocity = vector.new(),
            acceleration = vector.new(),
            expirationtime = 10,
            size = 2,
            collisiondetection = false,
            collision_removal = false,
            vertical = false,
            playername = "singleplayer",
            texture = {
                 name = "flame_tongue.png",
                 animation = {
                     type="vertical_frames", 
                     aspect_w = 16,
                     aspect_h = 16,
                     length = 1.6, --16 frames total at 0.1s/frame
                     loop = true, -- default to 'false' ?
                 }.
            },
            material_type_param = 0xdeadbeef,
            glow = "#AAAF"
        })
@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 1, 2016

Note that we have a bug that particles cannot be seen through translucent nodes, we have many issues with translucency so be careful with that.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Nov 1, 2016

And so in total:

  1. est31 request changes, approve them, but did not put approval label.
  2. sfan5 request changes but did not approve them.
  3. sofar against it.
  4. paramat against it.
    Unless est31 and sfan5 are willing to agree to merge this pull request, I don't see any reason to continue to edit it.
    @paramat particles cannot be seen through translucent nodes because translucent nodes rendered before particles, and therefore particles did not pass depth buffer test. To fix it we should render translucent nodes after particles, but in contrary it will result in ugly visual big, when parts of translucent nodes behind semi-transparent pixels will not be visible when they are behind them. To fix this z-buffer write in particles render should be always disabled. An order of visibility of particles will depend in that case from an order of a render and we must either ignore that fact or sort them by distance to a player before render.

@sofar particles in my pull request does not use vertical frames animation. They use two dimensional frame sheets with any number of rows and columns. I can't use tile animation definition, nor I want to limit animation only to one dimensional frame sheet.
Here is what I offer:

  1. Remove any mentioning of 'minetest.pack_texture_blend_func' to not scare "average
    mod writer" (oh, Satan, I hate those fucking pussies already!)
  2. Add a several pre-calculated values in documentation e.g.:
    0xMYEYESHURT - Additive blend
    0xICANTSEESHIT - Subtractive blend
    and so on. It not a real numbers in example, probably it will be a normal integers.

@paramat paramat added the One approval label Nov 1, 2016

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 1, 2016

Lol.
Added the missing approval label.

@sofar

This comment has been minimized.

Copy link
Member

sofar commented Nov 1, 2016

I'm actually in favor of this change, not against. My criticism is intended to make it even more palatable :)

I can't use tile animation definition, nor I want to limit animation only to one dimensional frame sheet.

Can you explain why the existing tiledef sheet is unusable? I don't understand why it works for animated node textures but not animated particle textures. In other words: if tiledef is broken by design, why don't we fix it, instead of introducing a new, incompatible API that is intended to do the same?

@sofar

This comment has been minimized.

Copy link
Member

sofar commented Nov 1, 2016

To add to my previous comment: It seems horizontal_frames could be implemented with [snip: irrelevant detail]

edit: well, not entirely just yet, but we should see whether this can get added separately and made to work.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Nov 1, 2016

@sofar

To add to my previous comment: It seems horizontal_frames could be implemented with just 4 lines of code:

There is no horizontal OR vertical frames in particle animation. There is horizontal AND vertical frames in particle animation. Here is an another test texture I used:

Hope it help you. Also you forgot about first_frame and loop_animation value. With first_frame and frame_lenght<=0 you can use any static frame from sheet as particle texture and using particle_spawner you can randomise this value. You can't do anything barely similar with tiles.
Do you want me to rewrite tile animation code completely? Those aspect_h and aspect_w values looks ugly as hell for me.

@paramat
BTW there is no "Irrlicht stuff" in Lua API doc anymore. It was removed, because est31 and sfan5 ask me to do so and we reached a compromise.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

@Zeno- although hmmmm is wrong about the Irrlicht dependency he still agrees with my points, so he still potentially has plenty of opposition. Foghrye4 is trying to trick people into thinking hmmmm's objections are now invalid.
You can't merge this with the amount of objection this has, we should wait for celeron55's opinion too since he is the 'conflict resolver'.
The issue is not the amount of documentation, it's the complexity of the blend options in something as simple as particles, when we don't have anything close to these options for nodes themselves, and when semi-translucency is avoided for nodes and everywhere else.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

I've sent a message to c55, let's wait for his input.

@Zeno-

This comment has been minimized.

Copy link
Contributor

Zeno- commented Nov 14, 2016

@paramat hmmm does not have the last say on merges!

@celeron55

This comment has been minimized.

Copy link
Member

celeron55 commented Nov 14, 2016

There are two ways this can go: Either we are ending up with a particle system that practically never needs any more work and everyone is happy with forever, or we are ending up with a complicated particle system that nobody even uses and is a pain in the ass to maintain. I don't know.

The code quality seems fine enough. That said, I haven't tried it out at all, and it seems more things other than particles could benefit from this work if somebody actually cared about things other than particles.

I do think the focus of this PR is maybe the weirdest ever but I guess people want particle effects then... I mean, do people want this? Maybe we should post on the forums and ask the general modder population what they think of this? It seems everyone so far is just guessing.

One thing is for sure: the documentation for the blend function is horrible, it needs to be rewritten in a way that goes from simple to complex. And spitting out magic integers and not telling where they come from is not simple. Give example uses of minetest.pack_texture_blend_func() right away everywhere or something.

So, I propose two things before a possible merge:

  1. fix the documentation,
  2. ask the modders.
@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

I have a request that may be acceptable to the author, something essential.
The blend API should be focussed on the majority of users who use the basic example values, not the minority using the technical blend parameters.
Currently casual modders will be using:
material_type_param = 12641,
Which is not acceptable API, the parameter name is technical and unclear, and the value used an abstract number.
I request that the parameter is called 'blend_type =' or similar, and that it also accepts values for the basic essential blend types that are clear English: "additive", "subtractive', "inverted".
This way the technical blend options are completely hidden for the majority of users.

I think the 'needs improvement' label is justified, it always has been, but even more so now.

min_first_frame = 0, -- <- for particle spawners
max_first_frame = 0,
-- ^ specify first frame to start animation.
frame_length = -1,

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Does this not conflict with:

+   length=3.0
+    --  ^ specify full loop length. 

?

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Hm i guess because we have 'first frame' and 'min/max first frame', no longer is the frame length decided by 'length / number of frames'. Do we need 'length' at all then, could animation speed be decided by 'frame length'?

#### Node animation, particle and particle spawners
* `{ type="vertical_frames",
aspect_w=16,
-- ^ specify width of a picture in pixels.

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

'frame' not 'picture'?

length=3.0
-- ^ specify full loop length.
first_frame = 0, -- <- only for particles, use
min_first_frame = 0, -- <- for particle spawners

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Do particle spawners select a random starting frame per particle between these two values? This should perhaps be documented.

This comment has been minimized.

Copy link
@Foghrye4

Foghrye4 Nov 14, 2016

Author Contributor

@paramat yes, I thought this would be obvious as min_pos and max_pos.

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Yeah it's somewhat obvious.

vertical_frame_num = 1,
horizontal_frame_num = 1,
-- ^ specify amount of frames in texture.
-- Can be used both for animation or for texture transform

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Confusing, what is 'texture transform' and how is that different from animation? I suspect it isn't.

-- See also "Particle blend".
animation = {Animation definition},
-- ^ see above. Note, that particle and particle spawners have differences.
glow = 15,

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

If not set i guess particle takes the light from the environment at that point? This should perhaps be documented.
If glow = 0 and particle is in a bright area does the particle take the higher value of 'glow' and environmental light? I'm not too bothered either way but just wondering.

This comment has been minimized.

Copy link
@Foghrye4

Foghrye4 Nov 14, 2016

Author Contributor

Particle brightness = min(environment light level + glow, 15).

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Hmm, interesting. @sofar what do you think? i remember you were asking for a fixed light level.

animation = {Animation definition},
-- ^ see above. Note, that particle and particle spawners have differences.
glow = 15,
-- ^ optional, specify particle self-luminescence in darkness.

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Comment implies darkness is required for glow?

This comment has been minimized.

Copy link
@Foghrye4

Foghrye4 Nov 14, 2016

Author Contributor

@paramat Yes. If environment light level is maximal this value will not affect particle brightness.

This comment has been minimized.

Copy link
@paramat

paramat Nov 14, 2016

Member

Ok i see.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

Zeno- i'm not 'blocking it because i don't understand it', i partly understand the blend functions. I am opposing for the specific reasons given, and i think i've been specific enough :] I can't block it anyway.

I'm not suggesting hmmmm has the final say on a merge. Experience had shown that he tends to spot things others don't, and he has the most knowledge of the current Minetest code. His opinion carries weight.

@Zeno-

This comment has been minimized.

Copy link
Contributor

Zeno- commented Nov 14, 2016

Ya know what paramat?

@Zeno- Zeno- merged commit 93e3555 into minetest:master Nov 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

@sfan5 and @est31 support this PR but insist on waiting for hmmmm's clarification, @celeron55 insists on improved documentation and further consideration. You made a mistake. Please revert.

@Zeno-

This comment has been minimized.

Copy link
Contributor

Zeno- commented Nov 14, 2016

paramat, you know I respect you. But you are wrong in this case. Let's move on.

@celeron55

This comment has been minimized.

Copy link
Member

celeron55 commented Nov 14, 2016

This has to be reverted; we don't merge when there is controversy.

sfan5 added a commit that referenced this pull request Nov 14, 2016

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

Ok now that's over i'll continue writing this:

To be clear i support the basic intent of this PR. I want particle animation and glow, i almost started coding glow myself. While i don't see much need for blend options i don't mind a few basic ones being available. I disagree with the technical blend options, but seeing the support for this i have little hope of stopping those. All i ask is that #4705 (comment) is addressed, then i won't be as opposed as before.

Obviously the documentation needs improving, plus any issues hmmmm brings up.
If i had not made a fuss over this it wouldn't now be improved and about to get better, it's annoying i know but for the best, i was willing to become a little unpopular to make sure the issues are addressed.

@Zeno-

This comment has been minimized.

Copy link
Contributor

Zeno- commented Nov 14, 2016

@Foghrye4 can you please create a new PR? I cannot reopen this one.

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Nov 14, 2016

@Zeno- yes, I can. _long sigh_

I wonder, what is easier - convince to merge something to Minetest or create 3D chunks mod for Minecraft?

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

3D chunks in Minecraft was the most interesting thing i ever saw in it, and was a condition for the voxel game i chose 4 years ago.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

@Foghrye4 hang on, see #4774

@Foghrye4

This comment has been minimized.

Copy link
Contributor Author

Foghrye4 commented Nov 14, 2016

@paramat they really done that? Last time I checked it was still WIP (3D chunks).

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

Well what i saw was a mod, and this was 4-5 years ago.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Nov 14, 2016

Ok, the mod i saw years ago may have only extended the height to a few thousand nodes.
Your link is the real thing: a cubic world.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.