Item light emitation based on volume (experimental. And depends on Craft Skill PR #595)

[AquariusPower]

copied specific changes from https://github.com/Attnam/ivan/pull/587

IMPORTANT: crafting PR has a fix to let it work with this PR properly, therefore THIS PR DEPENDS ON CRAFTING PR: https://github.com/Attnam/ivan/pull/595.

[AquariusPower]

I presume the emitation-by-volume only works for light-emitting materials (like light crystals), not items with scripted light emitation (like lanterns and some artifacts)? Because items with scripted light emitation shouldn't be affected by their volume.

I havent tested (neither knew) about scripted light emitation, as I remember, it will just check the main material volume. But scripted emitation should be ignored indeed, we just need to determine how it can be identified, there should have some IsScriptedLight() or something like that I guess.

The light seems to disappear completely when you have items <10g. Maybe we could put there a hard minimum, so there is always at least some light emitted? I liked how the ~10g piece of light crystal only cast dim light on my square, something like that could be the minimal light emitation.

It was initially intended as a tip to let the player know the minimum, but I think it is more annoying then fun.

Also, if I am not wrong, get 3 crystals, the red, the green and the blue. Cut -10g from each. Now I think one of them will be completely dark, like 10g for that one will provide less than the minimum light necessary to light the current square.

So, this is good too, have always a minimum no matter what volume, but if I am not wrong, I couldnt fully determine what could be a good minimum, based on that 3 crystals: red, blue and green. It seems the colors interfere on the final value that is used to decide the minimum emitation to light up the current square, I think it may require some more tests/research.

Btw, this is not exactly complete, many tiny light sources should sum up (10x 10g should be equivalent to 100g), but I think the vanilla game already do NOT do that. So it could be overly complicated to implement an inventory check for every light source and still grant the final result in the right place in the code.

I don't think this needs to be an option. Why not make it a full-blown change? :)

So we can default it to true (or just comment it)? I mean, if someone else says it should be an option, the implementation is already there xD.

Actually, initially the tests were very buggy, therefore severely broken, and it was messing the savegame, leaving squares that had nothing on them, still emitting light even after loading the game! The code used to let it finally work is the result of practical tests, basically I tested based on what was coded elsewhere until it worked, and it is not 100% good... we should really understand WHY it worked to be sure it wont fail later, so I think the main reason for the option is in case it fails (for any reason) in the future, may be a new item/material we havent tested yet when being splitted... I think that problem wont happen again, but unfortunatelly I can't grant it 100%.

I think this help info may be outdated/imprecise, as I remember fixing that problem: https://github.com/Attnam/ivan/pull/596/files#diff-fcabfb002c534b8efef6de0240e5e89fR179

[AquariusPower]

The complicated part was making "craft split item" not mess the map square.

This code is at void craftcore::FinishSpawning(recipedata& rpd,item* itSpawn){ https://github.com/Attnam/ivan/pull/587/files#diff-7fbccbc9b4f87c5e177faf8526c3bd4eR4076

currently as:

void craftcore::FinishSpawning(recipedata& rpd,item* itSpawn){
  itSpawn->MoveTo(rpd.rc.H()->GetStack());DBGLN;

  DBG3("EmitDbgSpawned",itSpawn->GetEmitation(),itSpawn->GetVolume());
  if(itSpawn->GetEmitation()>0){ //TODO is there a better way to do this emitation fix?
    // uses the previous emitation to fix everywhere before recalculating item emitation
    rpd.rc.H()->SignalEmitationDecrease(itSpawn->GetEmitation());
    rpd.rc.H()->CalculateEmitation();

    rpd.rc.H()->GetLSquareUnder()->SignalEmitationDecrease(itSpawn->GetEmitation());
    rpd.rc.H()->GetLSquareUnder()->CalculateLuminance();

    // last
    itSpawn->SignalEmitationDecrease(itSpawn->GetEmitation());
    itSpawn->CalculateEmitation();
  }
}

And it is the result of several tests, that in the end, only this way it worked w/o leaving problems on the map or the player's emitation, but I completely based it on elsewhere code, not on really understanding how emitation works...

I think this code is at my craftskill2 branch, and it is for sure at somefixes1234 branch, and I think you can only really test it using this PR that contains both codes: https://github.com/Attnam/ivan/pull/587 Or may be better combine the craft PR with this PR as they ended up being strongly related concerning fixing that nasty bug.

[AquariusPower]

Concerning the minimum light, I think it should be dark grey (r=g=b) therefore ignoring any specific coloring so that way may be more simple to let it work. This means a blue crystal of 1g would emit very dark grey light, what visually in the end may make no difference at all. I will see if I implement a way to test it using the developer console, therefore avoiding require to recompile just to know the minimum values.

[AquariusPower]

there is some code to test minimum light on somefixes1234 branch, use this line at devCons: DbgSetVar iEmitMinR 10;dbgsetvar iEmitMinG 10;dbgsetvar iEmitMinB 10 (tweak the values to test) but anyway, the bug below shall be fixed before we can even try to test for the minimum light values...

The bug seems to still exist... but it wont happen on the first cut/split... if you cut/split (thru crafting) a eg. blue crystal stone with -1, you will get a small stone that will have no light. (better go to a big empty place and just throw the rocks to the 8 directions to see their lights) now, split it again, you will get a stone with high light. now, split it again, you will become a source of light even without crystal stones. now, split it again, the map square may remain with light even having no light source. It is proving to be quite complicated to test and fix :(

[AquariusPower]

ok, I did some changes and a lot of tests, it seems to be working correctly now, the mentioned bugs look fixed, I couldnt replicate them after many splits of crystal stones.

It now has a minimum emitation, so any 1cm3 of any material that emits light will light up at least the current square it is on.

The changes are at https://github.com/Attnam/ivan/pull/587 for now, I will split them later to this PR and crafting PR (as this one depends on that one because of the fixes that one provides for the problem splitting creates).

[AquariusPower]

I still need to know if scripted lights got messed by this PR? (I still need to know what they are at all? how to identify them if needed) EDIT: I think a scripted light that may fail, is if it is on a dagger that has it's main material with less than 100cm3 that triggers the low light calculations.

but please test it using this branch for now: https://github.com/Attnam/ivan/pull/587, as it contains every new things, I mean, there is no point on testing light emitation w/o the crafting code. Or merge both crafting+light branches before testing :)

[red-kangaroo]

I meant this:

https://github.com/Attnam/ivan/blob/87f4a33fc84e8b3b432c9db4211c3d213433cc98/Script/item.dat#L149

It seems that it's probably not going to matter, because all items with non-zero BaseEmitation are bigger than 100cm3.

[AquariusPower]

btw, I am reworking how the light is calculated, it was not good... scripted lights will be skipped too. now the volume reference is 200cm3 (max for crystal stones), but can be easily changed. I will commit initially at somefixes1234 branch til it is ready.

[AquariusPower]

ok, I feel satisfied with the results now:

• all light emitters (least scripted ones) will have a minimum light for all color components
• max light is now from 200cm3 up, this means you can instantly see the light difference from crystal stones dropped after digging crystal shards (as they are from 100 to 200cm3).
• the best test I found is: grab some crystal stones, get one of them and cut -cm3: 1, 5, 10, 20, 50. Now spawn a small chest (throw it on wall to break lock), and start putting all crystals on it, from biggest to smallest to see the light changes differences one by one. Now do the inverse, take from smallest to biggest! :), and throw them around too!

Remember, merge crafting with this branch to properly test lights w/o all the fixed new issues/bugs.

PS.: not tested, crafting with crystals, eg. daggers (25cm3 on main material), other spawned items that already have light emitter on main material.

Should secondary material be considered too? we could have a dagger made of dark matter (main) and a hilt of blue crystal stone, and it would be cool! :)

[ryfactor]

Sorry to be a pain, but what happens if you cut a sol stone with negative volume?

[AquariusPower]

Sorry to be a pain, but what happens if you cut a sol stone with negative volume?

I never did that, and never saw a reason to do it, but I am sure someone will try it !!! xD This means I have no idea... it may crash? I think tho it could be prevented initially, if someone find a good reason to do it then we could implement such calculations, btw that is about crafting and not lights ;)

[AquariusPower]

if someone creates a "heavenly brightful golden sol stone" -100000cm3 (and proportional -weight too), it could be a good reason to let it be cut (with special calculations)! so it would also be a light emitter, and extra light calculations would be also required to make it work properly too. (unless sol stones could absorb light, but I am not sure such effect would be cool. But instead it could invert the lights, now that could be fun :))

Mostly considering we can now auto organize items into small containers by engraving eg. "+ring+wand" etc on the container (such when (auto)pickup will be auto stored, I think this code is only at somefixes1234 branch).

[ryfactor]

Oh wait, sol stones have negative mass, not volume. As a safety we could use the absolute value, or modulus, of the volume?

[AquariusPower]

I just spawned one sol stone (a hammer and a dagger), it has: -1000g -417cm3 :) I tested cutting it eg: -10 , nothing happened I tested splitting it in 2: one of the parts vanished! (bug) I tested splitting it in 10: all parts but one vanished! (bug) I think a simple test should be there for now: if volume is negative, deny doing it, so we avoid the bug (I dont know if there is something else with negative volume).

To work with negative value, may be set a boolean bIsNegative just before getting the absolute value to work with, and restore after, it would have to be placed at the recipe struct because restoring may be done after a crafting that lasts more than one turn.

[red-kangaroo]

Mostly considering we can now auto organize items into small containers by engraving eg. "+ring+wand" etc on the container (such when (auto)pickup will be auto stored)

That's pretty cool!

Could you please eventually document all these changes in NEWS (or MANUAL if they require longer explanation), so that the players (and us :D ) can learn about them?

[AquariusPower]

Mostly considering we can now auto organize items into small containers by engraving eg. "+ring+wand" etc on the container (such when (auto)pickup will be auto stored)

That's pretty cool!

Could you please eventually document all these changes in NEWS (or MANUAL if they require longer explanation), so that the players (and us :D ) can learn about them?

I tried to document it in the help, but it is a bit hard to see. The help F1 for auto-pickup option has some info about auto-storing in containers but that is a not very good place to have that info. I just thought we could have a tutorial/hints option that if enabled (default true) would trigger help messages as soon we pickup items that have "hidden" features, for eg.: When you first find any chest and pick it up, it will trigger a in-game popup help explaining about auto-storing feature. I think it may not be hard to code it, and is the best way to let players know about things they could otherwise not have a chance to benefit of. I dont know if there are other things we could add to the tutorial tho, but it is a start :)

[AquariusPower]

Could you please eventually document all these changes in NEWS

I will try to update it all at least at somefixes1234. btw, I changed auto store in container to use pcre regex (the way I was coding before was getting messy/complicated/tooLimited), I was trying to create a branch/PR for it but couldn't find all changes yet to make it work ,will try again later.

Ok, updated NEWS file (some stuff are probably missing), I think what is there should be split for every related PR if needed.

[AquariusPower]

My main concern about this PR is this code on crafting PR: https://github.com/Attnam/ivan/pull/595/files#diff-7fbccbc9b4f87c5e177faf8526c3bd4eR4225

void craftcore::FinishSpawning(recipedata& rpd,item* itSpawn){
  itSpawn->MoveTo(rpd.rc.H()->GetStack());DBGLN;
  DBG3("EmitDbgSpawned",itSpawn->GetEmitation(),itSpawn->GetVolume());
  if(itSpawn->GetEmitation()>0){ //TODO is there a better way to do this emitation fix?
    // uses the previous emitation to fix everywhere before recalculating item emitation
    rpd.rc.H()->SignalEmitationDecrease(itSpawn->GetEmitation());
    rpd.rc.H()->CalculateEmitation();

    rpd.rc.H()->GetLSquareUnder()->SignalEmitationDecrease(itSpawn->GetEmitation());
    rpd.rc.H()->GetLSquareUnder()->CalculateLuminance();

    // last
    itSpawn->SignalEmitationDecrease(itSpawn->GetEmitation());
    itSpawn->CalculateEmitation();
  }  
}

This code lets the decreased emitation be properly/correctly applied to moving sources of light, so, this code grants the emitation will not bugout and create glitches etc.

The point is, this was implemented based solely on tests, so what I mean is: It works but why? Why we have to use SignalEmitationDecrease()? What is the difference between CalculateLuminance() and CalculateEmitation()?

The problem of not knowing clearly why it works is the missing documentation (that I wasnt able to create) that would help on maintaining it if some problem happens later...

That's why this PR code is experimental and should remain as optional (could even default to enabled, but the user should be able to disable it).

So, just to make it sure, what is missing here is a documentation explaining why it works and how it works.

PS.: this could become an issue?

[ryfactor]

We will merge this right after #595 is merged.