Author Topic: New facotry smokes  (Read 473 times)

0 Members and 1 Guest are viewing this topic.

Offline Vladki

New facotry smokes
« on: October 09, 2017, 08:20:01 PM »
Hello James, I have noticed the new semitransparent factory smokes. They are nice, but they appear too low. E.g. the flare on oil well looks like if the tower itself is burning. And the same is with other factories (power plant, steel mill, textile facotry). As if it needs +1 or even +2 height levels.

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 15697
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: New facotry smokes
« Reply #1 on: October 09, 2017, 09:18:57 PM »
I did attempt to adjust the offset on these, but I could not find any reliable relationship between the specified offset and the position of the actual smoke in game; have you any experience of adjusting these such that they are correctly aligned that you are able to share to assist in improving the positions in Pak128.Britain-Ex?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Vladki

Re: New facotry smokes
« Reply #2 on: October 10, 2017, 09:13:59 PM »
I tried adjusting smoke on oil-well.  Offset=0,-32 makes an acceptable (but not perfect position). However it seems that the offset is scaled by pak size, i.e. 128/64, because the smoke appears 64 pixels higher than with offset 0,0

Horizontal offset +/- 32 will put the smoke over the right/left corner of tile, while 0 is at the center.
Vertical offset 0 is approx. at the top (rear) corner of the tile, -32 moves it one tile up, which approximately matches the height of tower on oil well. The horizontal offset is really tough to estimate correctly, because the smoke position is partially randomised, and _is_ affected by rotation.

offset = -4,-32 looks good too, but the difference caused by randomness is bigger than +/-4. It may be better to put the smoke even higher. So that its relative position to the tower/chimney is not that obvious.

Offline Vladki

Re: New facotry smokes
« Reply #3 on: October 11, 2017, 10:38:41 PM »
Sooo, I think I now understand how the factory smoke works. Really it sucks, as it is a legacy from the times when there were no rotations, and no animations. First: I have disabled the randomness, so that the smoke will appear consistently on the same place:

Code: [Select]
diff --git a/simfab.cc b/simfab.cc
index 7f5f93a..86a2d95 100644
--- a/simfab.cc
+++ b/simfab.cc
@@ -1630,8 +1630,10 @@ void fabrik_t::smoke() const
                koord ro = rada->get_pos_off(size,rot);
                grund_t *gr = welt->lookup_kartenboden(pos_origin.get_2d()+ro);
                // to get same random order on different compilers
-               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;
+               //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;
+               const sint8 offsetx =  ((rada->get_xy_off(rot).x)*OBJECT_OFFSET_STEPS)/16;
+               const sint8 offsety =  ((rada->get_xy_off(rot).y)*OBJECT_OFFSET_STEPS)/16;
                wolke_t *smoke =  new wolke_t(gr->get_pos(), offsetx, offsety, rada->get_images() );
                gr->obj_add(smoke);
                welt->sync_way_eyecandy.add( smoke );

Then I could start experimenting. Setting smoketile is easy. It is the tile with chimney. According to the 0th rotation in dat file, and this image:
e.g. for pharmaceutical factory it should be 1,1

Smokeoffset is where it sucks. For proper rotation in should be 3-axis coordinate - x+y to set the base of chimney within tile, and z for the height of chimney. But alas it is not so. X offset is on the screen X axis, and it goes from -32 for SW corner of tile through 0 (center) to +32 being the NE corner of tile. This is also properly rotated. Y offset is also screen offset, but is NOT rotated. So it is effectively height above the SW-NE tile axis. More over 0 is not ground level, but slightly above. So, if the chimney is close to the diagonal it works well, but if it is far away, it won't. Randomness can somewhat hide this fact, or putting the smoke higher so it is not so obvious that it far from the chimney.

So, after a few tests, I came with quite satisfactory offset for oil well (-6,-36) and (-8,-56) for pharma factory. hardware factory does not have a chimney so it should be smokeless.

Code: [Select]
diff --git a/industry/hardware-factory.dat b/industry/hardware-factory.dat
index a6a5404..ed625ef 100644
--- a/industry/hardware-factory.dat
+++ b/industry/hardware-factory.dat
@@ -25,10 +25,6 @@ electricity_amount=2
 electricity_boost=800
 MapColor=82
 sound=felix-blume-old-factory.wav
-smoke=Industry_Smoke
-smoketile=0,0
-smokeoffset=0,0
-smokespeed=5
 InputGood[0]=Bretter
 InputCapacity[0]=96
 InputFactor[0]=50 
@@ -90,10 +86,6 @@ electricity_amount=5
 electricity_boost=1000
 MapColor=82
 sound=felix-blume-old-factory.wav
-smoke=Industry_Smoke
-smoketile=0,0
-smokeoffset=0,0
-smokespeed=5
 InputGood[0]=Bretter
 InputCapacity[0]=128
 InputFactor[0]=50 
diff --git a/industry/oil-well.dat b/industry/oil-well.dat
index 2519b98..845bb60 100644
--- a/industry/oil-well.dat
+++ b/industry/oil-well.dat
@@ -27,7 +27,7 @@ MapColor=5
 sound=aifoon-chemical-factory.wav
 smoke=Industry_Fire
 smoketile=0,0
-smokeoffset=-8,-8
+smokeoffset=-6,-36
 smokespeed=3
 OutputGood[0]=Oel
 OutputCapacity[0]=350
@@ -91,7 +91,7 @@ MapColor=5
 sound=aifoon-chemical-factory.wav
 smoke=Industry_Fire
 smoketile=0,0
-smokeoffset=-8,-8
+smokeoffset=-6,-36
 smokespeed=3
 OutputGood[0]=Oel
 OutputCapacity[0]=600
diff --git a/industry/pharmaceutical-factory.dat b/industry/pharmaceutical-factory.dat
index 3cd324d..9eda5f4 100644
--- a/industry/pharmaceutical-factory.dat
+++ b/industry/pharmaceutical-factory.dat
@@ -25,8 +25,8 @@ electricity_amount=2
 electricity_boost=1000
 MapColor=153
 smoke=Industry_Smoke
-smoketile=0,0
-smokeoffset=0,0
+smoketile=1,1
+smokeoffset=-8,-56
 smokespeed=5
 sound=aifoon-chemical-factory.wav
 InputGood[0]=vegetables
@@ -116,8 +116,8 @@ electricity_amount=3
 electricity_boost=1125
 MapColor=153
 smoke=Industry_Smoke
-smoketile=0,0
-smokeoffset=0,0
+smoketile=1,1
+smokeoffset=-8,-56
 smokespeed=5
 sound=aifoon-chemical-factory.wav
 InputGood[0]=Chemicals

Adjusting the values is quite time consuming, so I'm not continuing further. I'll have a look at the code, if there is a decent chance to change the offset to 3D.


Offline jamespetts

  • Simitrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 15697
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: New facotry smokes
« Reply #4 on: October 11, 2017, 11:01:23 PM »
Splendid, that is very helpful - thank you for your work on that. I have now incorporated your changes.

The alignment is rather time consuming, as you note, so I suspect that I shall not have time to work on this further for a very long time indeed.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Vladki

Re: New facotry smokes
« Reply #5 on: October 14, 2017, 10:40:48 PM »

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 15697
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: New facotry smokes
« Reply #6 on: October 15, 2017, 03:41:51 PM »
Thank you for your work on this. Looking at the commit, you appear to remove the smokespeed parameter. However, this would mean that any existing pak files that have this defined would be read incorrectly, possibly leading to errors. Would you be able to modify the code so that "smokespeed" is retained in its previous format, having the new parameter as an additional, rather than a replacement, parameter? This will require incrementing the version of the factory writing code, which will need more work (i.e. a new section in the reader for reading this new type of code and setting the default for the new setting for older versions, etc.).

Edit: I also note that the pakset version of this also incorporates some changes relating to the production_per_field and storage_capacity of certain industries - may I ask what this relates to?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Vladki

Re: New facotry smokes
« Reply #7 on: October 15, 2017, 04:05:05 PM »
Thank you for your work on this. Looking at the commit, you appear to remove the smokespeed parameter. However, this would mean that any existing pak files that have this defined would be read incorrectly, possibly leading to errors. Would you be able to modify the code so that "smokespeed" is retained in its previous format, having the new parameter as an additional, rather than a replacement, parameter? This will require incrementing the version of the factory writing code, which will need more work (i.e. a new section in the reader for reading this new type of code and setting the default for the new setting for older versions, etc.).
I have put the smokeheight to the same position and integer type as was the previous (unused) smokespeed, so it should read the old speed as new height. The smokes position were broken anyway, so this should not make a big difference. I admit that I have no idea about where to set the version and defaults in reader. Can you point me to some example?

Quote
Edit: I also note that the pakset version of this also incorporates some changes relating to the production_per_field and storage_capacity of certain industries - may I ask what this relates to?

Yeah that belongs to the car factory. I just thought that visually the "fields" around car factory are just storage, not production facilities. So I was to experimenting with that, and it got comitted together with the fix for smoke on the car factory. Also I found out that the storage of fields is proportionally divided between all inputs and outputs. So if the base factory has storage 160+100+50 and two "fields" as part of the building, each additional filed should have capacity 310/2 = 155. (Pushed now to my branch).

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 15697
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: New facotry smokes
« Reply #8 on: October 15, 2017, 11:11:36 PM »
The problem with this approach is what happens if anyone has specified the documented (but apparently deprecated) "smokspeed" parameter: it would be read as if it were the new parameter, which might cause unexpected behaviour. It would be preferable to use the smokepseed parameter, as it is useful to be able to specify this, or at least reserve this for future use.

As an example of how to implement a new version, look at the history of versions. For example, factory_reader.cc has:

Code: [Select]
if (version == 4)
{
// Versioned node, version 4 with sound and animation
desc->placement = (site_t)decode_uint16(p);
desc->productivity = decode_uint16(p);
desc->range = decode_uint16(p);
desc->distribution_weight = decode_uint16(p);
desc->color = decode_uint8(p);
desc->fields = decode_uint8(p);
desc->supplier_count = decode_uint16(p);
desc->product_count = decode_uint16(p);
if (extended)
{
desc->pax_level = 65535;
}
else
{
desc->pax_level = decode_uint16(p);
}
if (extended)
{
desc->electricity_proportion = decode_uint16(p);
desc->inverse_electricity_proportion = 100 / desc->electricity_proportion;

desc->upgrades = decode_uint8(p);

if (extended_version > 3)
{
// Check for incompatible future versions
dbg->fatal("factory_reader_t::read_node()", "Incompatible pak file version for Simutrans-Extended, number %i", extended_version);
}
}
desc->expand_probability = rescale_probability(decode_uint16(p));
desc->expand_minimum = decode_uint16(p);
desc->expand_range = decode_uint16(p);
desc->expand_times = decode_uint16(p);
desc->electric_boost = decode_uint16(p);
desc->pax_boost = decode_uint16(p);
desc->mail_boost = decode_uint16(p);
desc->electric_demand = decode_uint16(p);
if (extended && extended_version > 1)
{
desc->pax_demand = 65535;
desc->mail_demand = 65535;
desc->base_max_distance_to_consumer = decode_uint16(p);
}
else
{
desc->pax_demand = decode_uint16(p);
desc->mail_demand = decode_uint16(p);
desc->base_max_distance_to_consumer = 65535;
}
desc->sound_interval = decode_uint32(p);
if (extended_version >= 3)
{
desc->sound_id = decode_sint16(p);
}
else
{
desc->sound_id = decode_sint8(p);
}

DBG_DEBUG("factory_reader_t::read_node()", "version=4, platz=%i, supplier_count=%i, pax=%i, sound_interval=%li, sound_id=%i", desc->placement, desc->supplier_count, desc->pax_level, desc->sound_interval, desc->sound_id);
}
else if(version == 3) {
// Versioned node, version 3
desc->placement = (site_t)decode_uint16(p);
desc->productivity = decode_uint16(p);
desc->range = decode_uint16(p);
desc->distribution_weight = decode_uint16(p);
desc->color = decode_uint8(p);
desc->fields = decode_uint8(p);
desc->supplier_count = decode_uint16(p);
desc->product_count = decode_uint16(p);
if(extended && extended_version > 1)
{
desc->pax_level = 65535;
}
else
{
desc->pax_level = decode_uint16(p);
}
if(extended)
{
desc->electricity_proportion = decode_uint16(p);
desc->inverse_electricity_proportion = 100 / desc->electricity_proportion;

if(extended_version >= 1)
{
desc->upgrades = decode_uint8(p);
}
else
{
desc->upgrades = 0;
}
if(extended_version > 4)
{
// Check for incompatible future versions
dbg->fatal( "factory_reader_t::read_node()","Incompatible pak file version for Simutrans-Extended, number %i", extended_version );
}
}
desc->expand_probability = rescale_probability( decode_uint16(p) );
desc->expand_minimum = decode_uint16(p);
desc->expand_range = decode_uint16(p);
desc->expand_times = decode_uint16(p);
desc->electric_boost = decode_uint16(p);
desc->pax_boost = decode_uint16(p);
desc->mail_boost = decode_uint16(p);
desc->electric_demand = decode_uint16(p);
if(extended && extended_version > 1)
{
desc->pax_demand = 65535;
desc->mail_demand = 65535;
desc->base_max_distance_to_consumer = decode_uint16(p);
}
else
{
desc->pax_demand = decode_uint16(p);
desc->mail_demand = decode_uint16(p);
desc->base_max_distance_to_consumer = 65535;
}
DBG_DEBUG("factory_reader_t::read_node()","version=3, platz=%i, supplier_count=%i, pax=%i", desc->placement, desc->supplier_count, desc->pax_level );
} else if(version == 2) {

(and so on): each of those "else if(version == X)" blocks of code specifies what data need to be read in what order for each different version. If you change the version, you will need to make an additional block of code along these lines and add the extra data that you want to be read. You will also need to change the version in makeobj. Note, however, that you should really change the Extended version if you are making this code just for Extended rather than also for Standard, but you should change the main version if you are intending the same code to work for both.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Vladki

Re: New facotry smokes
« Reply #9 on: October 17, 2017, 06:38:35 AM »
Ok, I think I will keep and use the smokespeed option, then. And perhaps add some more. Smokelifetime cones to my mind, as the current behavior is a bit odd. Producing factory should generate a steady smoke. Not just a puff every few minutes.




Offline jamespetts

  • Simitrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 15697
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: New facotry smokes
« Reply #10 on: October 17, 2017, 10:14:50 PM »
I notice that you have committed some further changes to the code, albeit I have not yet reviewed them in detail. May I ask whether you consider the current state of the code on your branch to be complete, or are you planning on doing more work on it in the immediate future?
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Offline Vladki

Re: New facotry smokes
« Reply #11 on: October 17, 2017, 10:19:39 PM »
It is not finished yet, I have some ideas but did not have time to implement them. Both in code and pakset.

Offline jamespetts

  • Simitrans-Extended project coordinator
  • Moderator
  • *
  • Posts: 15697
  • Total likes: 395
  • Helpful: 174
  • Cake baker
    • Bridgewater-Brunel
  • Languages: EN
Re: New facotry smokes
« Reply #12 on: October 17, 2017, 10:57:44 PM »
Splendid - let me know how you get on.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.