The International Simutrans Forum

Development => Patches & Projects => Incorporated Patches and Solved Bug Reports => Topic started by: Leartin on January 23, 2017, 02:51:16 PM

Title: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on January 23, 2017, 02:51:16 PM
Smoke in Simutrans is a good idea. Buildings are generally static, but if a factory is producing, it indicated that by a two-frame animation of smoke up the chimney - or similar two-frame-animation.

However, the good idea is kind of ruined by other advances Simutrans made, most importantly animation with no frame limit. With that, it's possible to have, say, an animated oilpump. Even smoke is sometimes better done as an animated frontimage - simply because there might be several chimneys, and you can only have one smoke per factory defined. Furthermore, Smoke is limited to factories only, while any building can have frontimages and animation, so using those is more consistent in the creation process.
There are still advantages to smoke - it is easier to use the same smoke in several buildings, and if it is reused, storage space in the pak format is smaller (since references are used, rather than the same thing stored individually for each building) And, of course, it's still the only thing that can be shown only if a factory works.

On that basis, I want to suggest the following changes:

1.) A new parameter "animate_when_active"
A parameter for factories, which is usually set to 0. However, if set to 1, the animation of the factory is frozen to the first frame as long as it is not productive.
Essentially, this enables animation to replace smoke as an indicator whether a factory is actually producing. Smoke can only add to a building,which would not work with the aforementioned oilpump, which would need to be visible at all time.

2.) No smoke-frame-limit
Currently, smoke only has two frames, which to me is the most important reason why I would not use it, except maybe for some blinking lights. If that was changed, I could see using it for several animations I currently use frontimages for. But there could be more:

3.) Enable smoke for all buildings
This is about smokes second ability: It is a graphic that's only referenced, not actually put in a pak-file. Most obviously, my endeavour to give most residential buildings some animated smoke in winter would greatly profit from such a possibility (if either the building or the smoke itself can be restricted to snowy winter). But I guess there are many small additions one could put in a smoke image just to have some tiny movements here and there.
Say, a smoke image with a hundred empty frames, and then a mole looks up through the floor. Could be places anywhere with some open ground, thus why not place it in several different buildings?

4.) More then one smoke per building
Most obviously, this would be nice if you have four chimneys on your coal power plant, so you could place a smoke on each. But also considering potential season-related animations (winter smoke of buildings) or rarely visible animations (mole from the ground) at some point, you would want to add some of them together.

5.) Allow the user to disable smoke
Now all these smoke-related animations are nothing but eyecandy, right? Nothing that is really needed, but could affect performance, so why do that to older computers? Well, usually there is no choice. You could hardly create a function to "disable all frontimages", since that would plainly be terrible. Stopping all animations, while working, is not ideal - imagine all these houses in winter with static smoke at their chimney. No smoke at all would be better, right? That means I either create it for nicer graphics on better PCs, or I don't, for the sake of older ones.
Now, smoke is a bit different. As said, in contrast to building animation, smoke only adds stuff to a building that already works without it. And that's how it should be: If a smoke reference is not found, it should not be replaced (nobody wants the mole to look out of the chimney), and the building should still be good, if not quite great. This is true for current smoke as well, obviously, since if not producing, factories won't have it.
So if it was disabled by the player, the gameplay would not change at all, just the graphics would not be quite as nice. And not all animation is lost - oilpumps would still pump on ;)
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Ters on January 23, 2017, 04:22:11 PM
Quote from: Leartin on January 23, 2017, 02:51:16 PM
2.) No smoke-frame-limit
Currently, smoke only has two frames, which to me is the most important reason why I would not use it, except maybe for some blinking lights. If that was changed, I could see using it for several animations I currently use frontimages for.

It is difficult to see that they are animated with all the frames due to them overlapping, but there are factories in pak64 that has smoke with more than two frames. When the same smoke is used for trains, it certainly has more than two frames. There are however factories with just two, or even just one, frame smokes.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on January 23, 2017, 05:10:43 PM
Quote from: Ters on January 23, 2017, 04:22:11 PM
It is difficult to see that they are animated with all the frames due to them overlapping, but there are factories in pak64 that has smoke with more than two frames. When the same smoke is used for trains, it certainly has more than two frames. There are however factories with just two, or even just one, frame smokes.
It's possible the restriction was once lifted, I did not try myself - I was still under the impression it would be two for buildings and five for trains.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: An_dz on January 23, 2017, 06:41:54 PM
Leartin's smoke must be a new smoke type. The current smoke is simply an image that moves on the screen, Leartin's idea is to make smoke as framed animation.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Ters on January 23, 2017, 06:46:20 PM
Quote from: An_dz on January 23, 2017, 06:41:54 PM
Leartin's smoke must be a new smoke type. The current smoke is simply an image that moves on the screen, Leartin's idea is to make smoke as framed animation.

No, smoke has been animated in Simutrans since at least 2008. That's how far back I can trace the image containing smoke animations in pak64.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on January 24, 2017, 12:29:47 AM
Smoke for factories and trains is the same. Hence those could easily have more than one frame of animation changed every 2.5 seconds. Than was introduced when raucher_t was changed into smake_t, which has been chnaged Nov 2006 (r452). The movement of the smoke is just in vertical direction, but the starting position is slightly random.

Currently factories are derived from buildings. As such each tile could have an animation independent from the other tiles, and the building does not know that it belongs to a factory. However, in principle stopping (and starting) animation is possible, one just need to remove/add the factory buildings to the animation list.

However, I am not sure the animation for factories when working goes the same way as the sound for factories when producing ...
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on January 27, 2017, 09:31:29 AM
Quote from: An_dz on January 23, 2017, 06:41:54 PM
Leartin's smoke must be a new smoke type. The current smoke is simply an image that moves on the screen, Leartin's idea is to make smoke as framed animation.

Let's say my idea is to use smoke as an excuse for a broader possibility to define animated graphics via reference, rather than normal animation/frontimage. Smoke just has a bunch of unique features, which I could see being useful elsewhere as well.

Quote from: prissi on January 24, 2017, 12:29:47 AM
However, I am not sure the animation for factories when working goes the same way as the sound for factories when producing ...
There is sound for factories? O.O First time I hear that.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on January 27, 2017, 12:35:37 PM
I made a patch, but after ambient sounds (waves for sea, birds for forest etc) were implemented but not used outside pak64 (or has that changed), I did not follow up on that. But it would be not too hard to add factory sounds.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on January 29, 2017, 01:47:39 PM
Quote from: prissi on January 27, 2017, 12:35:37 PM
I made a patch, but after ambient sounds (waves for sea, birds for forest etc) were implemented but not used outside pak64 (or has that changed), I did not follow up on that. But it would be not too hard to add factory sounds.

Maybe due to lack of documentation? Would not be the first time nobody uses a new function simply because nobody knows how, or that it even exists.

Either way, I think the obvious difference between 'animated when producing' and 'sounds when producing' is that there are already animations which could be changed to act like that. Eg. that animated bulldozer in one of pak64 factories. Sounds on the other hand would need to be found on the internet with a proper license and adjusted to fit in with other sounds, which isn't something pak-developers are familiar with in the first place.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 05, 2017, 05:08:51 AM
Only playing animation when producing was trivial. Also preliminary sound support added in r8067, new parameter sound_intervall (ms) and sound_id. Needs new makeobj ...
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 22, 2017, 09:03:05 AM
Since this thread already exists, might as well go here:

Yesterday I played around with smoke, since I learned there is no frame limit and transparency would require some changes to smoke eventually. I used this graphic for testing purposes: http://gushh.net/blog/wp-content/uploads/2011/06/smoke_1_40_128.png

Now I can confirm that the animation works, even with 40 frames and transparencies, but I came across 2 Limits:

First the buggish one: Smoke Size. It seems like the dirty-code for smoke is still hardcoded to 64px graphics, since I got a LOT of dirt.
Spawning and Despawning rates seem to be fixed for vehicles. While industry has a parameter "smoke_speed", it did not work with vehicles. Which means quicker vehicles are stuck with generating a little smoke-puff about every tile, with 5 of them existing at any one time. Shouldn't they be somewhat connected to create one long smoke-pipe along the route - so they would need to spawn more often?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Dwachs on February 22, 2017, 11:18:52 AM
To fix the dirty pixel bug: Can you upload a vehicle and smoke object, where this can be tested?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 22, 2017, 05:03:22 PM
Sure:
http://dl.dropbox.com/s/utn5ize8zk02moh/newsmoke.png (http://dl.dropbox.com/s/utn5ize8zk02moh/newsmoke.png) <-- Graphic
obj=smoke
name=Steam3
image[0-40]=newsmoke.<$0/8>.<$0%8>


https://www.dropbox.com/s/vpfrfcl7x7b7gaf/smoke.Steam3.pak?dl=0 (https://www.dropbox.com/s/vpfrfcl7x7b7gaf/smoke.Steam3.pak?dl=0) <- Pak file



Since this replaces normal smoke, pretty much any steam loco should work, eg. Adler - but the quicker the loco, the more artifacts you will see.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: TurfIt on February 22, 2017, 11:22:59 PM
Quote from: Leartin on February 22, 2017, 09:03:05 AM
First the buggish one: Smoke Size. It seems like the dirty-code for smoke is still hardcoded to 64px graphics, since I got a LOT of dirt.
Spawning and Despawning rates seem to be fixed for vehicles. While industry has a parameter "smoke_speed", it did not work with vehicles. Which means quicker vehicles are stuck with generating a little smoke-puff about every tile, with 5 of them existing at any one time. Shouldn't they be somewhat connected to create one long smoke-pipe along the route - so they would need to spawn more often?
Not hardcoded to 64, but hardcoded to expect the images don't differ in size much. Your humungous cloud broke that... Should be fixed with r8113.
Spawning is fixed to approx every 500ms, and each is limited to a life of 2500ms.

IMHO, that is way too large of a cloud, and especially if you're using alpha, that'll kill the game in early years when lots is on the map.
I did notice an anomaly where the smoke spans multithreaded rendering regions, perhaps due to how far away from the vehicle the smoke is appearing - 4 tiles, or maybe because the cloud itself is so large.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 23, 2017, 08:47:13 AM
Quote from: TurfIt on February 22, 2017, 11:22:59 PM
IMHO, that is way too large of a cloud, and especially if you're using alpha, that'll kill the game in early years when lots is on the map.
I did notice an anomaly where the smoke spans multithreaded rendering regions, perhaps due to how far away from the vehicle the smoke is appearing - 4 tiles, or maybe because the cloud itself is so large.

But it would kill the game looking fabulous! ;)
Yes, this is too much. But you need to know what's possible, and I learned with this test. Besides, as long as spawning is fixed, no smoke will look too good anyway, even this just looks silly on fast trains with 5 seperated clouds.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Dwachs on February 23, 2017, 04:16:00 PM
Quote from: TurfIt on February 22, 2017, 11:22:59 PM
I did notice an anomaly where the smoke spans multithreaded rendering regions, perhaps due to how far away from the vehicle the smoke is appearing - 4 tiles, or maybe because the cloud itself is so large.
This is not related to multithreading, it is a bug in the clipping stuff.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Dwachs on February 24, 2017, 04:06:22 PM
The problem was that certain portions of tiles were drawn twice. This is a problem if alpha transparency is involved.

I committed a fix to this problem with r8116. I am not completely sure that this did not break anything else. Please watch out for clipping errors.

These clouds are fantastic!
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: jamespetts on February 24, 2017, 04:08:38 PM
May I ask under what license that these clouds are released and whether they might be made available under the Artistic Licence to enable their inclusion in Pak128.Britain(-Ex)?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: An_dz on February 24, 2017, 05:41:02 PM
I highly believe it's CC-BY-SA 3.0, but it's better to wait for a confirmation from Leartin.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 25, 2017, 06:56:45 PM
Sorry - as I said they were a testing graphic, not intended to be actually used! I am not the creator of these clouds, they were made by GuShH.

However, there license is actually quite simple: "You may use them for private or freeware (open source only) projects. Commercial use is prohibited, please contact me regarding licensing or custom work." - that's it, completely compatible with Artistic License. Not quite compatible with CC-BY-SA.

Just use the one I linked, since they are actually 128p, rather than my resized ones from the Dropbox.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: TurfIt on February 25, 2017, 07:44:16 PM
Quote from: Leartin on February 23, 2017, 08:47:13 AM
Besides, as long as spawning is fixed, no smoke will look too good anyway, even this just looks silly on fast trains with 5 seperated clouds.
What do you suggest is required to allow it to look good?
Parameters for spawn rate? lifetime? spawning not tied to time? animation rate?
What about the fixed movement currently applied? Parameters for speed/direction of movement?  (IMHO any movement needs to be contained to the tile [or maybe a slight overrun cheat...], but I certainly don't want to see smoke hopping tiles)
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 25, 2017, 11:12:25 PM
Quote from: TurfIt on February 25, 2017, 07:44:16 PM
What do you suggest is required to allow it to look good?
Parameters for spawn rate? lifetime? spawning not tied to time? animation rate?
What about the fixed movement currently applied? Parameters for speed/direction of movement?  (IMHO any movement needs to be contained to the tile [or maybe a slight overrun cheat...], but I certainly don't want to see smoke hopping tiles)
Lifetime is just animation rate times framecount, it is virtually the same. I'd go with lifetime for backwards compatibility, It's not required for train smoke I think, but nice to have for factory smoke.

If spawning was not tied to time, but to the distance moved, you might run into the opposite problem of what we have now - a really slow locomotive might create smoke so rarely, one cloud would be gone before the next spawns. If you go by time, but use a higher spawning rate so the smoke looks good on faster locos, it might be too thick on slower locos, and not exactly performance friendly if it draws more than needed. Especially considering that the slower the trains, the more can fit the screen, the more smoke can be on screen.
Ideally, there would be both. I'd like to define the highest allowed distance travelled before a new smoke particle spawns (eg. half of a tile), but if that distance is not travelled within a certain amount of time, spawn a particle anyways (eg. every 500ms). If something like that is too complicated, I think the next best would be a timebased spawnrate defined by the train, so quicker trains can have a higher spawnrate for the same smoke object.

The fixed movement, you mean going up a few pixels? I think that does not need to be part of the smoke behaviour at all. It can be done within the graphics themselves. If there is just one graphic, like in p64 factory smoke, it could still be defined manually with an offset. So I would scrap that as a game behaviour and put it in the pak creators hands instead, since they already have the tools to recreate it and there are generally not many different smokes per pakset,so it's reasonable to adjust to.
Since we are at it, I'd like to define the offset radius of smoke. The original is good for p64, but barely noticable in p192c. I'd rather have either none or a bigger one.

Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: TurfIt on February 26, 2017, 04:35:40 AM
Quote from: Leartin on February 25, 2017, 11:12:25 PM
Lifetime is just animation rate times framecount, it is virtually the same. I'd go with lifetime for backwards compatibility, It's not required for train smoke I think, but nice to have for factory smoke.
Or animation rate is framecount divided by lifetime as it currently is!


Quote from: Leartin on February 25, 2017, 11:12:25 PM
Ideally, there would be both. I'd like to define the highest allowed distance travelled before a new smoke particle spawns (eg. half of a tile), but if that distance is not travelled within a certain amount of time, spawn a particle anyways (eg. every 500ms). If something like that is too complicated, I think the next best would be a timebased spawnrate defined by the train, so quicker trains can have a higher spawnrate for the same smoke object.
Adding a spawn time and max distance moved between spawns can be done. A fast moving vehicle with a low framerate would result in missed spawns - need to not set that max distance too low, or spawn too short. I'd rather stick with the mechanism where the spawn occurs at the vehicles current position at the end of a frame, than try to calculate the spawn positions more accurately on a strict time/distance basis.


Quote from: Leartin on February 25, 2017, 11:12:25 PM
The fixed movement, you mean going up a few pixels? I think that does not need to be part of the smoke behaviour at all. It can be done within the graphics themselves. If there is just one graphic, like in p64 factory smoke, it could still be defined manually with an offset. So I would scrap that as a game behaviour and put it in the pak creators hands instead, since they already have the tools to recreate it and there are generally not many different smokes per pakset,so it's reasonable to adjust to.
Yes, I meant the fixed upward motion rate. I presume you're meaning to have multiple animation frames with different offsets? I think keeping graphics fixed, and moving in the code makes more sense. Isn't that the current proposal for the animated pedestrians?


Quote from: Leartin on February 25, 2017, 11:12:25 PM
Since we are at it, I'd like to define the offset radius of smoke. The original is good for p64, but barely noticable in p192c. I'd rather have either none or a bigger one.
Can you expand on what you mean here? Currently factory smoke has tile and pixel offsets, vehicle smoke is fixed position.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 26, 2017, 08:34:44 AM
Quote from: TurfIt on February 26, 2017, 04:35:40 AM
Or animation rate is framecount divided by lifetime as it currently is!
Lifetime divided by framecount, but I agree - Which is why I said I would go with lifetime

Quote from: TurfIt on February 26, 2017, 04:35:40 AMAdding a spawn time and max distance moved between spawns can be done. A fast moving vehicle with a low framerate would result in missed spawns - need to not set that max distance too low, or spawn too short. I'd rather stick with the mechanism where the spawn occurs at the vehicles current position at the end of a frame, than try to calculate the spawn positions more accurately on a strict time/distance basis.
Sorry, I don't think I understand that. You mean if the spawn happens based on travelled distance, the train may skip that exact position and no smoke would spawn? If so, tht could be a problem. Would it be any better if smoke does spawn on time, but the time shrinks shorter depending on vehicle speed? Something like up to 70km/h, it's 500ms, from 70 to 110 it's 350ms, from 110 upward it's 200ms. It could not drop any spawns if it's timebased, right?


Quote from: TurfIt on February 26, 2017, 04:35:40 AM
Yes, I meant the fixed upward motion rate. I presume you're meaning to have multiple animation frames with different offsets? I think keeping graphics fixed, and moving in the code makes more sense. Isn't that the current proposal for the animated pedestrians?
Pedestrians are moving entities. There is one animation loop, which is repeated while they walk around via code.
Smoke is not a moving entity, it stays in place. The upward motion is a relic from earlier time, when neither animation nor offset were available, and it worked to animate even one-frame-smoke a tiny bit. Just take a look here:
(http://dl.dropbox.com/s/qpzojp82pd9ssdt/smoke.png)
This is the smoke I use for common buildings in winter. It's just a 3-frame repeated pattern in the frontimage of the building. Smoke in a style similar to this could be used for factories as well. except it would automatically move upward 10 pixels, so I would need to create 30 frames and change the graphic every 10 frames, and increase the offset every 3 frames as a countermeasure.
On the other hand, if those ten pixels were abandoned, the one-frame-smoke of pak64 would change it's definition from
Image[0]=ls-smoke.1.0
to
Image[0-9]=ls-smoke.1.0,<$0>,0
and be exactly the same as it was before. That's why the movement needs not be in the code: If you want it, it's super easy to do manually, but if you don't want it it's harder to work around it.

Quote from: TurfIt on February 26, 2017, 04:35:40 AM
Can you expand on what you mean here? Currently factory smoke has tile and pixel offsets, vehicle smoke is fixed position.
Smoke spawns at slightly random positions. Very slightly, you can count the pixels with your fingers - best seen in p64 with factory smoke. I created a sheepfarm a while ago, with the word "BAA!" randomly popping up now and then. Except it's not random, it's a 30-second animation loop with 3/4 empty frames. If I could set the random offset of the smoke, I would instead use a "BAA!"-smoke at a really low spawning rate with high potential offset. Or the other way around, if I created an animation loop, like the building smoke, I might not want any offset to occur, since it breaks the animation.

Remember, "smoke" is the name of the object, not a restriction on what is allowed to do with it. The less limitations, the more possibilities. :)
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on October 14, 2017, 10:39:49 PM
Hello all,

I'm reviving this topic about smokes. As industry smokes were added to pak128.britain-ex, I found out that it is impossible to properly position the smokes. It seems that the code was made when there were no animations and no rotations. So If you positioned the smoke for one rotation, it will be way off in all others. The problem is in the offset being only 2D, which cannot be properly rotated. It has to be 3D to work well. I have looked at the code, it seems to be the same in standard and extended (maybe just some formatting changes). I have made a patch for extended, which you can see here: https://github.com/jamespetts/simutrans-extended/pull/81/files

The changes are these:
- removed option: smokespeed - it was not used anywhere in the code.
- smokeoffset is now the offset of chimney _base_. (0,0) is the center of the tile, (32,0) is the right corner, (0,16) is the bottom corner.
- new option: smokeheight is the height of chimney in pixels for pak64.  For pak128 it has to be divided by 2, etc.. Just like the smokeoffset. Default value is 8, which was formerly hardcoded in the code (wolke.cc).
- factory smoke is not randomised any more. It was just weird to see smoke appear sideways from chimney.
- also vehicle smokes benefit from this change. If a vehicle was climbing a slope and the map was rotated (and paused) smoke was displayed wrong, now fixed.
- smokeheight for vehicles is hardcoded to 8 (just as before, but in vehicle/simvehicle.cc).

I plan to further improve the smokes by reintroducing smoke speed and smoke randomness. At the moment they are implemented as constants in wolke.cc, but I'd like to make them into dat file options for the smoke itself. With smokespeed=0 the animation (and any movement of smoke) would be fully made in graphics. Randomness allows the smoke to wiggle a little bit in x axis. It works, but just sometimes looks weird. I'm not sure it it is really worth including. However I have no idea how to implement new options for smokes. They seem to be so simple objects that their reader/writer seems to be just some default (inherited from some simple generic object).

So please have a look, try it if you can, and tell me what you think about it.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on October 15, 2017, 12:42:28 PM
Awesome!

But I'm a bit concerned about the 3D coordinate, since that only works if the rotations are actually exact rotations of the same 3D building. In Pixel-Art, the easiest way to get a "rotation" is to mirror the base image and change the shading, a mirror image is not the same as an actual rotation, so the chimney would not be in the same spot.
Wouldn't it be smarter to do something like "smoke[x ]=", where x is the rotation? Hence you would be able to set individual smokes for each rotation. This would be especially useful since getting rid of randomness and (I assume) automatic rise would unlock the use of smoke in animation in general, eg. items on conveyer belts, people walking around corners, etc - all those kinds of animations that should not be visible at all when no production occurs, since seeing any frame without-motion would not make much sense. And unlike smoke, those could vary greatly between rotations.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: jamespetts on October 15, 2017, 01:15:57 PM
I suspect that it would be better ideally to have both alternatives, since simply using a 3d co-ordinate would be much more efficient for paksets that are rendered rather than hand-drawn, although one then has to consider the amount of work necessary to create and maintain two different systems.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on October 15, 2017, 04:31:25 PM
I must say that I have not coded in C++ for almost 15 years, so I was aiming for the simplest possible fix that will at least make the current stuff working properly.
I have not tested what it does if the industry has only 1 or 2 rotations (mirrored).
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on October 15, 2017, 05:34:55 PM
Quote from: jamespetts on October 15, 2017, 01:15:57 PM
I suspect that it would be better ideally to have both alternatives, since simply using a 3d co-ordinate would be much more efficient for paksets that are rendered rather than hand-drawn, although one then has to consider the amount of work necessary to create and maintain two different systems.
How efficient is it, though? Even if you get the 3D coordinate directly from blender (and you should - extrapolating it from 2D renders later would be a pain) you'd still need to break down the x and y data into tiles and and position on that tile, in a coordinate system thats 45° turned (assuming you use axis parallel to tile borders in the 3D program) While that process could be automated with some tool/plugin, it would be just as easy to get the 2D coordinates for 4 rotations as getting a simutrans-3D-coordinate...
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on October 15, 2017, 06:40:06 PM
First I must say that these are not 3D coordinates in isometric sense. The smokeoffset is in screen x,y pixels, with 0,0 being the middle of tile (for pak64 it is pixel 32,48; for pak128 = 64,96), and it is scaled by paksize/64, so the corners of tile are (32,0), (0, 16), (-32,0), (0, -16). Then the height of chimney, again in pixels divided by paksize/64. In any case it is easier to count them from the final render. However it is far from perfect. I have tested the code on rendered oil-well for pak128.britain-ex, and the base of the tower is jumping +/- 1 pixel in different rotations. Given the fact that the offset is multiplied by 2 (128/64), and during the rotation again multiplied and divided by 2, this gives us 4 pixel precision. So this low precision, gives much better chance for pixel art to end up more precise if you manage to put the chimney in the right place on all rotations. Now I also understand why adjusting the smoke on car factory was such a pain - it is not rotated but mirorred.

So, the real solution would really be either to have a separate offset for each rotation, or to get rid of smoke completely, and just make the factory animation dependent on production.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on October 15, 2017, 07:36:08 PM
Quote from: Vladki on October 15, 2017, 06:40:06 PM
In any case it is easier to count them from the final render.
If the base is visible. If it isn't visible, you are out of luck for an easy solution.
Quote from: Vladki on October 15, 2017, 06:40:06 PMSo, the real solution would really be either to have a separate offset for each rotation, or to get rid of smoke completely, and just make the factory animation dependent on production.
Actually, factory animation is only played during production for a while now. This works fine for something like an oil pump or a windmill that just stops in it's motion, but it's really strange to have a static smoke over a factory while it isn't producing. That's also why I'm interested in using smoke for other animations that would look strange when just stopped in motion, eg. flames, people, open octopus eyes over closed eyes when nothing happens...

Larger scale project: What if you could reference a smoke-object with the same syntax you use for images, since it is nothing but a collection of images? This would also allow for multiple smokes in one building (one per tile), different smokes depending on season, and you could play different smokes one after the other by using the animation index. While it's a bad idea to overuse that (for performance reasons) it would probably give the most options on how to use it.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on October 15, 2017, 09:44:22 PM
does the animation of factory just freeze, or is it reset to 1st frame? If the latter, then jsut make the 1st frame of smoke clear.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on October 16, 2017, 04:01:21 AM
It just freezes, and it starts out at a random frame to begin with.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Ters on October 16, 2017, 05:51:23 AM
Quote from: Vladki on October 15, 2017, 06:40:06 PM
So, the real solution would really be either to have a separate offset for each rotation

I think that would give artists the most amount of flexibility.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 22, 2020, 11:24:26 PM
Hello, I'm reviving this topic - I want to get the smokes right. And found some weird things about OBJECT_OFFSET_STEPS constant...

This constant is defined to be 16 at the end of simconst.h, with a comment that lots of other code would have to be changed if OBJECT_OFFSET_STEPS is changed.
In several places I found a code where something is multiplied by OBJECT_OFFSET_STEPS and immediately divided by 16. Why is that? I have a bad feeling that in some places I would need to do that (to cover the unlikely case when OBJECT_OFFSET_STEPS would not be 16, but I don't know how to find those... Either is this redundant, or would have to be added to other places, like when dealing with roadsigns and offset for left hand driving.
Can anyone shed more light on this?

simfab.cc, line 1548:

const sint8 offsetx =  ((rada->get_xy_off(rot).x+sim_async_rand(7)-3)*OBJECT_OFFSET_STEPS)/16;
const sint8 offsety =  ((rada->get_xy_off(rot).y+sim_async_rand(7)-3)*OBJECT_OFFSET_STEPS)/16;


simobj.cc, line 185

sint8 byte = (sint8)(((sint16)16*(sint16)xoff)/OBJECT_OFFSET_STEPS);
file->rdwr_byte(byte);
xoff = (sint8)(((sint16)byte*OBJECT_OFFSET_STEPS)/16);
byte = (sint8)(((sint16)16*(sint16)yoff)/OBJECT_OFFSET_STEPS);
file->rdwr_byte(byte);
yoff = (sint8)(((sint16)byte*OBJECT_OFFSET_STEPS)/16);


simroadtraffic.cc, line 180. There is a code that for versions 99.5 - 99.17 does not do this conversion.

if(file->is_version_less(99, 5)  ||  file->is_version_atleast(99, 17)) {
sint16 dummy16 = ((16*(sint16)hoff)/OBJECT_OFFSET_STEPS);
file->rdwr_short(dummy16);
hoff = (sint8)((OBJECT_OFFSET_STEPS*(sint16)dummy16)/16);
}
else {
file->rdwr_byte(hoff);
}


Then two less obvious places, where it is not divided by 16  but 4096 (>> 12) or 256 (>>8)

./obj/wolke.cc line 108, and 132
const sint8 new_yoff = base_y_off - ((insta_zeit * OBJECT_OFFSET_STEPS) >> 12);
set_yoff( base_y_off - ((insta_zeit*OBJECT_OFFSET_STEPS) >> 12) );

simvehicle.cc line 1236
wolke_t* const abgas =  new wolke_t( get_pos(), get_xoff() + ((dx * (sint16)((uint16)steps * OBJECT_OFFSET_STEPS)) >> 8), get_yoff() + ((dy * (sint16)((uint16)steps * OBJECT_OFFSET_STEPS)) >> 8) + get_hoff(), desc->get_smoke() );
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 23, 2020, 08:00:32 AM
The only thing I can say for sure, is that version 99 and older had a much worse movement code. Each tile was exactly 16 steps, which looked very ugly in larger pak sizes (even pak128).

But this is not relevant for your case. You need to extend makeobj and the descriptor (so there is more than one offset save in memory).
Instead
SmokeTile=1,0
SmokeOffset=0,-98
SmokeSpeed=12

you need to look for
SmokeTile[0]=1,0
SmokeOffset[0]=0,-98
SmokeSpeed[0]=12
SmokeTile[1]=0,1
...

and convert the old entires (in makeobj and in Simutrasn when loading old paks, if there is a rotation). Then, when rotating, you just take the new offset from the extended descriptor.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 23, 2020, 11:13:12 AM
Thanks prissi, but that is not what I asked for...

Before implementing multiple smokes per industry (or any building), I want to get the single smoke right. And for that I need to understand the what is going on with the values. Why they are at several places multiplied by OBJECT_OFFSET_STEPS just to be immediately divided by 16, which returns essentially the same value?

The only place I would expect this to happen is when reading the pak file, to scale the offset from 16 steps/tile used by pakset author to the current value that was compiled. But then all calculations should be done with whatever step size is defined. So I would expect it in load/save/reader methods, which is the case of simobj.cc, simroadtraffic.cc, but not in simfab.cc

The code in simvehicle.cc (dx * steps * OBJECT_OFFSET_STEPS) >> 8 seems to be conversion between VEHICLE_STEPS_PER_TILE (256) and OBJECT_OFFSET_STEPS (16), so it should be:
(dx * steps * OBJECT_OFFSET_STEPS) >> VEHICLE_STEPS_PER_TILE_SHIFT

Also in wolke.cc the shift ">> 12" seems to be just some arbitrary value set to "look good" in comparison with max. insta_zeit being another magic constant 2500.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 23, 2020, 03:10:29 PM
Quote from: Leartin on October 16, 2017, 04:01:21 AM
It just freezes, and it starts out at a random frame to begin with.

Sorry for double post. But wouldn't it be easier then to make the animations to start always at the first frame, and return to first frame if stopped? Then you could do everything just using animations. That should not be that complicated as fixing the smokes
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 26, 2020, 09:06:57 PM
I found another weird thing that breaks the smokes behavior on rotation.
Why is there the: if(xoff!=0)? It breaks rotations of stuff that has xoff zero but yoff not zero...

Simobj.cc:

void obj_t::rotate90()
{
        // most basic: rotate coordinate
        pos.rotate90( welt->get_size().y-1 );
        if(xoff!=0) {
                sint8 new_dx = -2*yoff;
                yoff = xoff/2;
                xoff = new_dx;
        }
}


Also what sort of disaster could happen if the xoff and yoff would be changed from sint8 to sint16 ?   y_offset 128 is not enought for some high chimneys in pak128.britain :-(
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Mariculous on February 26, 2020, 10:33:18 PM
Quote from: Vladki on February 26, 2020, 09:06:57 PMAlso what sort of disaster could happen if the xoff and yoff would be changed from sint8 to sint16
In the rotation itself, there should not be any disasters (I would not expect yoff >= 2^15 to happen)
simobj_t::rdwr: We do some calculations here that will overflow if xoff>=2^12 Further, we will need to ensure we save and load the 16 bit in newer versions, whilst loading 8 bits for older versions.
Sidenote: We actually calculate 16*xoff/16 there, so this seems strange anyway.

In further places spread around the code, however, there will happen some disasters. Most of them simply being truncating the sint16 to sint8, some further overflows might also happen

In any case, "Simply" changing this to 16 bits will cause many truncation disasters spread around the code. Some further disasters unknown type might also happen.
The following functions need some adjustments or considerations. This is not a complete list. There might be further functions involved called from one of the listed functions:
obj_t
  ::display
  ::display_after
karte_t::sync_step,
objlist.cc compare_trees
world_view_t::internal_draw
baum_t
  ::finish_rd
  ::recalc_off
  ::rotate90
roadsign_t::set_dir
road_user_t::rdwr
air_vehicle_t::display_after
vehicle_t
  ::display_overlay
  ::make_smoke
  ::rdwr_from_convoi
  ::rotate90

There are a few more occurences that won't need changes.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 27, 2020, 12:31:14 AM
Since you have millions of objects on large maps, and you break the packed alignment structure, you may suddenly end up with double memory consumption and slower code due to more cache misses. Furthermore, smoke_desc_t has the offsets as coordinates, which are sint16. So one could just spawn on the right tile (like supporting height as offset too in smoke_desc_t) and all things are solved. Nowhere wolke_t assumes to be on the map ground.


void fabrik_t::smoke() const
...
koord3d ro = rada->get_pos_off(size,rot);
grund_t *gr = welt->lookup(pos_origin+ro);
...


So smoke_desc_t::get_pos_off must return a koord3d instead of koord, which is rather trivial.

I will change the smoke_desc_t write tonight to used different 3d coordinates if there is an entry "smoketile[0]"=x,y,z and also use then up to four offsets, if "smokeoffset[0]" is given.

So the only change will be to smoke_desc_t::get_pos_off and handling of old style paks (which is like now) and new style, using an array of koord3d.

However, without seeing any code, this is totally blind guessing, and may be unfair to what you actually doing. (Which I have no idea).
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 27, 2020, 04:21:42 PM
Please wait with changes, I have almost finished patch fir exteded which could apply to standard too. I'll post later when I'm at pc. I tried to put the smoke at koord3d above ground but it was still shown on ground. Please look at related topic about factory smokes in extended forum
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 28, 2020, 01:01:22 AM
Kindly point me to the thread please. There is nothing obvious discussion this from the title. Anyway, I did it last night.

EDIT: Forgot the patch, which was only tested with old pak objects
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 28, 2020, 09:32:40 AM
Please, see here: https://forum.simutrans.com/index.php/topic,17456.msg185173.html#msg185173
And the patch here: https://github.com/jamespetts/simutrans-extended/pull/81

I spend several nights debugging and fixing the smokes, and just didnt have time to announce it, And I was not at computer yesterday evening either.
I see your patch is quite extensive and I fear that all my work went down the drain... :-(

Edit: looking more at the patch, yeah - your and mine patch touch many of the same places with different apporach:

It is not enough to specify the tile with koord3d, the offsets have to be 3d as well for the rotations to work well.
And I failed when tried to modify the koord3d pos when creating smoke, it was still displayed on ground level (see my post above).

My patch also makes smokelifetime konfigurable (with default 2500/2499), and also makes the smokespeed to work (it was ignored before).
Further plans are to make wind speed (horizontal movement of the smoke) configurable.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 28, 2020, 12:52:53 PM
Since the graphics could be very different for different rotations (especially when there are only two rotations of a factory which is 2x2), the only way is to specify proper offsets. A single pair of x,y will always fail because it cannot guess correctly, which direction the smokestack goes in rotation. Hence the smoketile parameter. (And the same for smokeoffset.)

Configurable times for smoke can be added too, indeed. And the smokespeed too. But at least in standard the smoke speed (meaning how much smoke in old versions per time) is tied to the production rate of factories and the speed of vehicles.

However, currently the whole smoke system is somewhat broken, since the smoke is not use a independent smoke descriptor but rather something in between. Hence, I am very reluctant to add too much further hacks on it (which would be required by a configurable lifetime etc.)

So here a corrected version.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on February 28, 2020, 08:33:18 PM
By smokespeed I mean how fast it rises up, not how often a new cloud is generated... Smokespeed 0 keeps the smoke on the same place so the artist can have full control of the movement.

Smoketime is good when adjusted to be the same as the duration of sound produced by the factory.

Smokeheigt is already hardcoded as 8 so I just made it configurable to make the rotation as precise as possible.

I know that even in rendered pak128.britain, some rotations are not exact, so the chimney and smoke are not always on the same place, but mostly it works fine.

For factories with 2 rotations, which are most probably just mirrored, a special rotation can be done.

If I understand your idea correctly you would prefer to specify the offset for each rotation separately and just make the smoke disappear on rotation?

I won't be able to compare your patch until Sunday evening, so please don't commit it. I spent quite a lot of time working on my patches and I would be quite disappointed if it would be useless.  Especially if your patch does not fix all what I fixed. Also I have already added the new options to several factories in pak128.britain and it looks really good. So I would like to keep that work... The only thing I need now is to overcome the height limit of sint8.

Also this is not only about factory smokes but also about vehicle smokes. They needed some fixes too.

I'm learning simutrans code this way so it takes me more time to find what's doing what. But when I find it I want to go in depth and prepare perfect patch.

I know that for some situations an animation would be better, that may follow later.

I also think about more smokes per factory...
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 29, 2020, 03:14:00 AM
Smoke is currently the only object, which does not have its own descriptor, since it is derived from two smoke objects, which were just images. Therefore, the whole thing is a complete mess. There are two ways out: Either make this more messy be adding more parameter (like you did) or make a smoke its own descriptor, at least for factories. And I personally would much more prefer the latter. Like factory_smoke_t derived from wolke, which has all then all correct offsets and thus know on rotation to which tile it has to change to.

The old smoke was made so it worked with the available factories. New factories were added in such a way, that the smoke came out in the right position upon rotation. Any factory emitting wrong smoke was not made correctly (apart from overflow issues).

But, if one goes for renovation then up to four locations of smoke for each of the four rotations are mandatory. There is absolutely no rule stating that all rotations have to show the same factory (and some paks do not show exactly the same). So the only proper way of dealing with this is imho  having four different offsets. (That was actually even an extension request a while ago.)

However, if there are four different offsets, then the smoke would have to disappear upon rotation, as it cannot guess its position correctly. Or it will smoke from somewhere. (Which is currently the case with smoke on tiles != (0,0) and which is not easily fixable, even with your patch. And just drawing with larger offsets on the wrong base tile is also not helping, because then part of the building can be drawn on top of the smoke at certain rotations.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: makie on February 29, 2020, 08:07:31 AM
Quote from: prissi on February 29, 2020, 03:14:00 AMBut, if one goes for renovation then up to four locations of smoke for each of the four rotations are mandatory.
I think we need four images too. Think about a factory with two or more not centered chimney, may be a tiny and a big one. Then the image have to rotate too.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on February 29, 2020, 12:02:15 PM
All the more for an independed object, or a changed definition of the factory descriptor.

(Originally, smoke was shared, but that would be almost impossible with such additions.)
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on February 29, 2020, 02:04:46 PM
Shouldn't we also talk about which (if any) new parameters would be part of the smoke, and which would be parameters of the object spawning the smoke?

The way I see it, all a smoke object needs to be is an array of images displayed one after another as an animation, that can be spawned pretty much independently from everything else. If that's not fancy enough, a smoke could also display one of several animation whenever it spawns, or each frame of the smoke could be the result of a particle generator. Basically, no matter how fancy, the smoke object would define what is displayed *

The other thing is the definition when and where to spawn. For vehicles, that's hardcoded. For buildings, the dat needs to define it. But if we think of the smoke as just a displayed animation, and multiple animations per factory are already on the scope of what we want to do, I really recommend reading the proposal in this thread: https://forum.simutrans.com/index.php/topic,18388. (To recap: Define smokes exactly like images, since just like images, we only need to know which goes where)


Lastly... how about making this a completely new thing? If the old smoke just stays as it is, and this new system would be independent from it, there would not need to be any consideration towards backwards compatibility. Eg. Particles don't need to move up 5 pixels during their lifespan, because if you wanted them to do that, you'd always do it with the graphics. This means if you want the smoke to move up 7 pixels during it's lifetime, you could do that by redrawing it seven times, rather than 12 times **

I'm not sure, but would the images displayed over a building as a smoke require more processing than the same images displayed as a frontimage? If not, switching over to use smoke for tiny, repeated animations would be really useful!


* Just to be clear: That's completely independent. Even with no changes to smokes as they currently are, I don't see why you couldn't just reload new images created by a particle generator in the 'slots' of a normal smoke to make it completely random any time. Of course, the particle generator needs to be made and hooked up, but it's completely independent from this discussion.

** If someone can't follow: If you had a seven-frame smoke currently, where each frame moves up one pixel, after a fifth of the lifespan, it would move up. Since you don't want that, you have to manually define a different frame that looks the same, but is one pixel lower. This is the case for 5 of the seven frames, which are different images, hence 12 frames. This is assuming the game is smart enough not to redraw the smoke if it neither moved nor got replaced with a different image; otherwise a seven-frame-smoke would require 35 redraws.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 02, 2020, 09:25:40 PM
OK, I agree that having separate position and offset for each rotation is better...

For what leartin says about the animations - in my patch I have added two options:
smokespeed - it says how fast the smoke moves up, with 0 - does not move at all, and 1 the current speed
smokelife - how long the smoke lives with 2500 being the current hardcoded default.
Also if you make the animation with more than 5 images, they will change even if the smoke does not move up.
I has a discussion with Jamespetts whether smokespeed (and smokelife) should be bound to smoke or to factory.
Smokespeed should be imho defined together with smoke, but for smokelife it is good to match the duration of sound emitted by factory.

Prissi, could you post an example how a dat file for factory with smoke should look like with your patch? I'll try to apply it to some factories in pak128.britain to see how it works.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 02, 2020, 11:47:58 PM
I think positions should be bound to factories. A derived factory_wolke_t from wolke_t could then just (upon rotation) look for a factory on its tiels, and then determine its new tile after rotation from its descriptor. (Same for lifetime, unique to factory_wolke_t.)

About smookespeed: Both options are fine, i.e. a dedicated parameter or not.

Having offsets as part of the factory would be even better for reusing the smoke uopn different buildings.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 03, 2020, 07:55:33 AM
So wolke_t would be the smoke from vehicles?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 03, 2020, 12:59:33 PM
wolke_t would stay like it is (or maybe renamed to smoke_t at this time), and a new factory_smoke_t is derived which uses the offset system for drawing the smoke and speed and lifetime and more checks for rotation.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 03, 2020, 09:49:48 PM
Prissi, please could you show an example how the dat file of factory with smoek should look like with your patch? Is that already with different offsets for different rotations?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 04, 2020, 01:49:30 PM
The dat file would look the same, but the smoke offsets will be stored with the factory and the smoke offsets of the smoke will be ignored. If this is an old factory pak, then after loading the smoke offsets will be copied from the smoke descriptor.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 04, 2020, 08:53:11 PM
Sorry I do not understand... What is your diff then doing? Only allowing y_offset to be sint16 ? Adding different offsets for  rotations ? How are they coded? So again How do I code smoke for factory that has very high chimney and 4 rotations ?

OK, I think I got it...   4 rotations: smoketile[0-3] and smokeoffset[0-3], and 16 bit y_offset...   I had to do some modifications:

in factory_writer:

- if( !obj.get( smoketile ) )
+ if( obj.get( smoketile ) == "" )
...
- if( obj.get( smoketile ) )
+ if( obj.get( smoketile ) != "" )


Also the write offsets were wrong:

                // now we know the size
-               obj_node_t node( this, 3 + (4 + 4)*rotations, &parent );
+               obj_node_t node( this, 3 + 8*rotations, &parent );
                node.write_sint16( outfp, version, 0 );
                node.write_sint8( outfp, rotations, 2 );
                for( int i = 0; i < rotations; i++ ) {
-                       node.write_sint16( outfp, st[ i ].x, 5 + 9*i     );
-                       node.write_sint16( outfp, st[ i ].y, 5 + 9*i + 2 );
-                       node.write_sint16( outfp, so[ i ].x, 5 + 9*i + 4 );
-                       node.write_sint16( outfp, so[ i ].y, 5 + 9*i + 6 );
+                       node.write_sint16( outfp, st[ i ].x, 3 + 8*i     );
+                       node.write_sint16( outfp, st[ i ].y, 3 + 8*i + 2 );
+                       node.write_sint16( outfp, so[ i ].x, 3 + 8*i + 4 );
+                       node.write_sint16( outfp, so[ i ].y, 3 + 8*i + 6 );



And found the weird condition in rotation90 in simobj.cc, is needed for bridges. With this patch, bridges (the ramps) get broken, at least in simutrans-extended. But we do not need that if the smokes are removed upon rotation.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 08, 2020, 03:08:35 AM
Bridges were broken, that hack in rotation was just a catastrophy wating.

I submit my changes together with your ideas and input about speed going upwards i r8948. This allows now to have a smoketile[0] and smokeoffset[0] (and further roations) parameter, to have smoke coming out at different locations, as well as an smokeuplift and smokelifetime which are default 16 and 2599.

Thank you very much, without your work this would have never been finished.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 08, 2020, 07:57:20 PM
Thanks, I'll try to backport (or forward-port) this to extended.  BTW with all the changes now, would it be too complicated to have multiple smokes per factory? In pak128.britain, there are several factories with multiple chimneys.  I'm not too configent in C++ so I don't know what would be the "right way". I see that the rotations are stored as vector, so would it then be a vector of vectors?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 09, 2020, 01:23:15 AM
This is a little more complicated, since the smokes are references semi-automatically resolved by the ref reader. But in principle, yes, there could be four different smokes per rotation (but more than one smoke per factory does no make sense, rather making a special smoke for that factory makes sense).
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on March 09, 2020, 01:30:03 PM
Quote from: prissi on March 09, 2020, 01:23:15 AM(but more than one smoke per factory does no make sense, rather making a special smoke for that factory makes sense).
Could you explain why?
One of the best parts about smokes (and why I'd wish for them in non-factory buildings) is that they are reusable. Sure, if there are, say, two chimneys, I can make a smoke with graphics of two smokes. But then, I'd need to use four of those if the building with two chimneys rotates, and that smoke could not be reused in other factories, so I'd start questioning why it would even be a seperate object.
It makes more sense to me that each smoke would have the same 'graphical origin', that is, if smokeA is replaced with smokeB, it's a different graphic, but still looks like it comes out of the chimney, not 20px left from it. I'd think smokes should be independent from specific buildings whenever possible.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Mariculous on March 09, 2020, 01:44:53 PM
Without knowing the code, I do in principle agree with Leartin. Multiple smokes per industry make much more sense than a single smoke per factory that might be a factory dependant "multismoke" frames.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 09, 2020, 11:36:43 PM
One can make smoke a part of the building, instead of only a factory without too much work and not introducing too much hacks. But having more than one smoke per building/factory and rotation will be quite some effort, which I doubt to ever see used in a real game for more than one object. (At least comparing to just copy smoke together into one image.)
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 22, 2020, 07:14:06 PM
Hello Prissi,

I have tried the new version, but there is a problem in descriptor/writer/factory_writer.cc (I wrote about it earlier).

The test for new/old style of specifying smokes is using: if(  obj.get( str_smoketile )  )
But in the definition in dataobj/tabfile.h says that:          * @return const char * returns at least an empty string, never NULL.
So the test is always true, and thus old style smoke definition (which is OK for factories wit only one rotation), does not work - smoke is generated at tile 0,0, offset 0,0
The needed change is either:

--- a/descriptor/writer/factory_writer.cc
+++ b/descriptor/writer/factory_writer.cc
@@ -277,11 +277,11 @@ void factory_writer_t::write_obj(FILE* fp, obj_node_t& parent, tabfileobj_t& obj
        sint16 const smokelifetime = obj.get_int("smokelifetime", DEFAULT_FACTORYSMOKE_TIME );
        char str_smoketile[] = "smoketile[0]";
        char str_smokeoffset[] = "smokeoffset[0]";
-       if(  obj.get( str_smoketile )  ) {
+       if(  obj.get( str_smoketile ) != ""  ) {
                for( int i = 0; i < 4;  i++  ) {
                        str_smoketile[10] = '0' + i;
                        str_smokeoffset[12] = '0' + i;
-                       if( !obj.get( str_smoketile ) ) {
+                       if( obj.get( str_smoketile ) == "" ) {
                                break;
                        }
                        pos_off[ i ] = obj.get_koord( str_smoketile, koord( 0, 0 ) );
-                       if( !obj.get( str_smokeoffset ) ) {
+                       if( obj.get( str_smokeoffset ) == "" ) {
                                dbg->error( "factory_writer_t::write_obj", "%s defined but not %s!", str_smoketile, str_smokeoffset );
                        }
                        pos_off[ i ] = obj.get_koord( str_smoketile, koord( 0, 0 ) );



Or, as I observer in other parts of code:

--- a/descriptor/writer/factory_writer.cc
+++ b/descriptor/writer/factory_writer.cc
@@ -277,15 +277,15 @@ void factory_writer_t::write_obj(FILE* fp, obj_node_t& parent, tabfileobj_t& obj
        sint16 const smokelifetime = obj.get_int("smokelifetime", DEFAULT_FACTORYSMOKE_TIME );
        char str_smoketile[] = "smoketile[0]";
        char str_smokeoffset[] = "smokeoffset[0]";
-       if(  obj.get( str_smoketile )  ) {
+       if(  *obj.get( str_smoketile ) ) {
                for( int i = 0; i < 4;  i++  ) {
                        str_smoketile[10] = '0' + i;
                        str_smokeoffset[12] = '0' + i;
-                       if( !obj.get( str_smoketile ) ) {
+                       if( ! *obj.get( str_smoketile ) ) {
                                break;
                        }
                        pos_off[ i ] = obj.get_koord( str_smoketile, koord( 0, 0 ) );
-                       if( !obj.get( str_smokeoffset ) ) {
+                       if( ! *obj.get( str_smokeoffset ) ) {
                                dbg->error( "factory_writer_t::write_obj", "%s defined but not %s!", str_smoketile, str_smokeoffset );
                        }
                        pos_off[ i ] = obj.get_koord( str_smoketile, koord( 0, 0 ) );


Or as used in way_writer.cc:          if (!strempty(obj.get(buf))) { ...

UPDATE:
Also the vehicle smokes are broken after rotation. I tried to put back part of the original code, which helped on straight track, but it was still broken when the train was going up- or down-hill. It is impossible to get the rotation right without proper 3D offsets (x_off, y_off, h_off), as was implemented in my proposed patch. So if 3D offsets are not wanted, we can discard the smoke after rotation altogether.  If the vehicle smoke after rotation is wanted, then I can prepare a patch for it.

Also if the game is paused and rotated, the industry smoke IS rotated and stays in place until cursor goes over it.   

Another question is about the calls to mark_image_dirty() in wolke.cc. Shouldn't the second parameter (yoff) be always -8  to match with set_yoff( -8 ); from constructor? It seems that set_yoff is not used anywhere else, as the movement is done in display_after().

Also the new_yoff calculation in sync_step is imho useless, it works fine like this:
@@ -116,14 +116,10 @@ sync_result wolke_t::sync_step(uint32 delta_t)
                return SYNC_DELETE;
        }
        const image_id new_img = get_front_image();
-
-       // move cloud up
-       const sint16 new_yoff = base_y_off - (((long)insta_zeit * uplift * OBJECT_OFFSET_STEPS) >> 16);
        if(  new_img != old_img  ) {
-               // move/change cloud ... (happens much more often than image change => image change will be always done when drawing)
                if(  !get_flag( obj_t::dirty )  ) {
                        set_flag( obj_t::dirty );
-                       mark_image_dirty( old_img, new_yoff );
+                       mark_image_dirty( old_img, -8 );
                }
        }
        set_flag( obj_t::dirty );





Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 23, 2020, 07:52:34 AM
The last bit will not work with long raising clouds, because only the base is marked dirty, not the raidsing cloud. I may work in your tests, since the dirty boxes are larger than the clouds. It was not also not noted so much, because even non-moving clouds were dirty all the time.

I tested mainly 64 size pak, and the vehicle smoke was fine for three out of four rotations. Since more smoke (for steam engines) is better than less, I just left it in. It is possible, that vehicle smoke in pak128 works less after rotation. (In pak128.britain is seems to cope even better with rotation.) But the smoke is only shown for a second or so after rotation before it fades anyway. So I decided this a non serious problem. Better than no smoke at least.

I mean rotation is something almost not player does frequently (or in networkgames never) and thus I would not bother too much. One could indeed move the industrysmoke to the right tile, but that will be a big hassle, since it cannot be done during the rotation, but only during hte next sync_step. (Since we cannot alter the tile during rotation! That is why the smoke stay when paused.)

about obj.get, sorry, that is indeed my fault. Corrected in r8966 (and the dirty offsets in r8967)
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 23, 2020, 10:10:01 PM
Hi, I have noticed, that sometimes the last sprite of smoke stays in place. Soot falling on the ground...

I tried to add yoffset in to the mark_image_dirty() in destructor  ~wolke_t(), but that did not help. I also added mark_image_dirty() call just before return SYNC_DELETE. That seems to be better, but occasional leftowers still happen

As the uplift calculation is already 3x in the code, I made a method calc_yoff, that does this calculation.
Further I found a bug in factory_writer.cc, that ignored offsets[0-3]. And some typos and fixes in comments
And as last I added a fix for smoke rotation to make it work perfectly at least on flat track.

Please see the attachment
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 27, 2020, 10:37:19 PM
I did further work on this.

0. xy_offsets were ignored due to a typo in factory_writer.cc
1. implemented perfect rotation for vehicle smokes - this requires 3D offsets, which are easy for vehicles. Factory smoke is discarded on rotation so there is no need to change anything.
2. fixed the mark_image_dirty calls (yoff must be scaled to tile_raster), so now the smokes properly disappear
3. removed the randomization of initial position of smoke
4. added a "wind" effect - the clouds move randomly sideways
5. some comment fixes

New patch attached, please ignore the previous one.
EDIT: screenshot of the "wind effect - #3 and #4"
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 28, 2020, 12:47:46 PM
Also the obj better rotates, even for factory smoke, otherwise it removal from the map may fail (if a deletion happens before the next step, i.e. roation after pause).

About the wind, there is more discussion needed. 2 pixel may be a lot for smaller smoke. And offset must be changed after marking dirty, or remainder of clouds are left.

Incorporated in r8980, thank you.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 28, 2020, 01:29:19 PM
Quote from: prissi on March 28, 2020, 12:47:46 PM
About the wind, there is more discussion needed. 2 pixel may be a lot for smaller smoke.

rand(2) returns either 0 or 1, and it is invoked only on yoff changes, so the wind is never too strong

And one more question, I'd like to get rid of that hard coded yoff=-8, at least for new factory smokes, so that the offset can be directly measured from the png image. Is that OK?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 28, 2020, 02:22:07 PM
If you can do it so the old is still intact, why not. But the offset is not in pixel anyway (unless you have 128 size).
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 28, 2020, 03:12:54 PM
Quote from: prissi on March 28, 2020, 02:22:07 PM
If you can do it so the old is still intact, why not. But the offset is not in pixel anyway (unless you have 128 size).
I'll prepare a patch. (And in pixels it is for pak64 ;)

Btw how often is the git synced from svn ? I do not see the latest patches in git.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on March 28, 2020, 11:54:24 PM
I hope final patch is here. The LEGACY_SMOKE_YOFFSET = -8 is now applied only to vehicle smoke and smokes defined the old way (no indexes). Indexed smokes have no default offsets. So you can simply measure the distance from top of chimney to the bottom of tile, i.e. SE corner, not (as I thought) from center of tile. Then measure the distance from the bottom of smoke to the bottom of tile, and subtract these two. Then divide by 2 for pak128, or by 3 for pak192, etc. That is the y-offset. (it is always negative). X-offset is measured from centre of tile, and can be +/- 32. Setting smokelift = 0 is good for debugging and checking if the offsets are correct.

I also used VEHICLE_STEPS_PER_TILE_SHIFT in simvehicle.cc instead of magic >> 8.

Here is video with the wind effect: https://uran.webstep.net/~vladki/simutrans/smoke.mp4
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Yona-TYT on March 29, 2020, 12:19:37 AM
I love it ... This looks very beautiful!. <3




An off topic question, is it regarding the wheels of the vehicles, can they also use animations? It will be great for pak192.comic.

Cheers !.
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on March 29, 2020, 02:25:48 AM
Git is synchronised once a day I think. I have no corntrol over the git repro. But incorporated your changes in r8990, thank you.

Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on March 31, 2020, 09:26:50 AM
Vladki, since you say it's final, would you mind writing a summary of all smoke parameters and what they do? Or perhaps provide a sample with all parameters in use?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Leartin on April 01, 2020, 12:23:46 PM
Quote from: Vladki on March 31, 2020, 08:18:04 PMEvery puff starts at exactly the same spot. Randomized is the wind effect instead.
-Is the wind effect dependent on uplift? That is, the more a smoke moves upwards, the more it can move sideways? Or would even a smoke without uplift move to the sides?
-wind effect is only sideways, right? so it will not simulate wind to northwest by increasing  how high smoke rises, or wind to southeast by decreasing it?
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: Vladki on April 01, 2020, 01:24:00 PM
Wind has no effect if uplift=0
It affects only x-offfset, so that means the wind blows from North-East. That is consistent with airplane takeoffs and landings.
X-offset is changed only when y-offset is changed, so the faster it goes up, the faster if would go sideways.

Uh... I just keeep wondering where my previos message was gone:  https://forum.simutrans.com/index.php/topic,19714.0.html
Title: Re: Changes to the smoke-parameter for buildings (and ideas around that)
Post by: prissi on April 01, 2020, 01:45:53 PM
Could you please stop discussing in this thread and start a new one in a place (like the techincal documentation where I moved your discussion). This here is considered a non active posting area, since nobody will look at search resilts here. So I will close here.