Jump to content


Photo

My patches and contribution


  • Please log in to reply
95 replies to this topic

#26 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3657 posts

Posted 12 September 2014 - 10:58 AM

What the scripting language does syntax wise,and how it is implemented internally are two different things. Actually, the scripting language does support bool:

bool foo = true; if (foo) { // do something }


True. (no pun intended). bool is a synonym of "float" right now I think. Most scripts I've seen ignore it and declare variables that they mean to use as booleans as floats.

I knew someone would object to the list of keywords. We already have such a sitatuion with "-" and "" - both are used as "don't use" but the code not always recognises "-" as empty.

Yeah, that is messy. It's probably a bit too late to fix it now. I've been stuck wanting to suppress an inherited spawnarg, but there's nothing you can do in DR if the spawnarg isn't one that recognises "-" as a null value.

As for "strToBool()", well, the current getBoolKey() is incomplete, too. It only supports "1" and "0". But sometimes people might "true" in spawnargs, or options. Parsing such values in the scripting language is very cumbersome and slow (multiple passes between scripting and C++). But I can take it out, if you wish. Emulating it in script will be deadslow, but possible.

Mapping "true" to boolean true isn't confusing or controversial. I thought the other mappings were unhelpful though, since they'd have to be documented and looked up. But even "true" wouldn't work consistently for spawnargs would it? You'd end up with people being able to use "true" for spawnargs that get read by scripts, but not being able to use it for spawnargs that get read by the game code, and with no easy way to tell the difference.

EDIT: crossed with your update. Let's see what others think. Mine's only one opinion. Or did you change your mind about it?

Edited by SteveL, 12 September 2014 - 10:59 AM.


#27 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 12 September 2014 - 12:16 PM

True. (no pun intended). bool is a synonym of "float" right now I think. Most scripts I've seen ignore it and declare variables that they mean to use as booleans as floats.


Yeah, it is a mess on both the scripting side (where people use float instead of boolean) as well on the C++ side, where sometimes floats are passed in and back, or sometimes ints and so on.

As develper, I found it especially confusing that there is no real way to pass a bool to a scripting event function - or at least nobody used one before. People use "int", which means if the scripting side uses something like "myFunction( true )", I think it converts that as "1.0f", then passes it along, then converts it to "1" which then gets cast by the function internally again as bool. Sometimes this is shortcut by using 'd' (int), but not always. What a mess..

Part of the mess is, I believe, that id itself developed that stuff while they did go along, a few things were later added (by yours truly among others :) and also people often just copy examples. Especially on the scripting side, as there were no good tutorials. Even now I often discovered stuff by accident.

One place with "float = getBool" and everyone just copies that line. The C++ code has the same issues.

And of course trying to fix that is quite a lot of work - I don't think we have the manpower for full rewrites or refactoring, so my idea was to fix it one step a time.

But since my main goal is actually not trying to fix up the TDM innards, I happily let it be :)

Mapping "true" to boolean true isn't confusing or controversial. I thought the other mappings were unhelpful though, since they'd have to be documented and looked up. But even "true" wouldn't work consistently for spawnargs would it? You'd end up with people being able to use "true" for spawnargs that get read by scripts, but not being able to use it for spawnargs that get read by the game code, and with no easy way to tell the difference.

EDIT: crossed with your update. Let's see what others think. Mine's only one opinion. Or did you change your mind about it?


Yeah, sorry, if that didn't come across as clear as it should. Mapping a fixed set of strings to true/false seems not the right thing, the list will ever be incomplete (is "open" = true? is "-" false" ? and so on. This will only ever create problems. And a routine that converts "1" and "0" to true/false is of not much use, one could just do bool B = StrToFloat("1"); and it would work. Looks not as clean. but heh, nobody cared before :)

So, I concur with your opinion on not adding StrToBool() at this time.
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#28 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 12 September 2014 - 12:26 PM

Funny, the code already knows about 'b'_

					    case 'b':
							    i = va_arg(args, int);
							    Push(i);
					    break;

game/script/Script_Interpreter.cpp line 2006.

It is just used nowhere in the code for event definitions :rolleyes:
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#29 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3657 posts

Posted 12 September 2014 - 01:08 PM

Funny, the code already knows about 'b'_

...

It is just used nowhere in the code for event definitions :rolleyes:


Nicely spotted. So we could have used it, but by now there's a pile of code that says "float activateTargets", that kind of thing. Whenever I see "float" in a declaration I'm so used to checking whether it means bool that I've stopped noticing. Not an argument against improving it, I know, but there's so much code already written and it works well.

I just noticed another Linux patch above. Cheers for that, I'll sort it when I've made the animation changes I've been testing the last day or two. Plus the new string handling functions. If you've raised trackers would you pin them to me please?

#30 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 12 September 2014 - 02:20 PM

Nicely spotted. So we could have used it, but by now there's a pile of code that says "float activateTargets", that kind of thing. Whenever I see "float" in a declaration I'm so used to checking whether it means bool that I've stopped noticing. Not an argument against improving it, I know, but there's so much code already written and it works well.


Guess we just have to disagree on the "works well" part :) It works internally, but it is a mess and causes further mess by not being explicit.

And yes, not only changing the attributions, but also putting that into the declaration would be my goal. So even the script event doc should say "returns bool" and "SomeFunc(bool doSomething)". I'll start a new thread about the issue of "int and bool in scripting" and post there a rough outline of what the changes and steps could look like.

Not that I want to implement them, but if we can come up with a plan, then any possible implementation becomes easier.

Edit: I just would like to add that this issue irks me on the "cleanlyness" level. Using a float instead of a boolean feels wrong, not only from the performance side, but also from the correctness side. There are tons of routines that want a boolean, but accept an int/float, and then proceed with the info they get. There might be a lot of bugs in the code lurking where bad things happen if you pass in unexpected values like "-3". Esp. the idStr routines all do either things like "strlen" (that can get you easily buffer overflows when things aren't zero-terminated), or just assume some number is either > 0 or -1, but never check for -3. And so on. That would be worthy of tightening and explicit parameters are first, small step. Of course, from the "heh it works" POV it, well, works. TDM 2.02 is the proof that it indeed works :)

I just noticed another Linux patch above. Cheers for that, I'll sort it when I've made the animation changes I've been testing the last day or two. Plus the new string handling functions. If you've raised trackers would you pin them to me please?


Thanx for looking at it. I haven't made a tracker for the linux issue (its a two-letter change). And neither for the string functions, should I do so?

The trigger_touch issue is tracked here http://bugs.thedarkm...iew.php?id=3823 and I just assigned it to you - guess you meant that by "pin"?

Edited by Tels, 12 September 2014 - 02:25 PM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#31 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3657 posts

Posted 13 September 2014 - 06:36 AM

Quick update on the various strands:
  • Linux compilation fixes: I've committed fix #2 under the same tracker as #1. I can see how this typo happened. VS2010 insists in displaying the /ai/ folder as /AI/. I tried changing it under project properties, which works in vs2013 but not vs2010, which immediatelty reverts to AI. Hence developers are likely to make mistakes when including it. The other folders seem to be displayed with correct case, so I don't know why AI is wrong.
  • String handling script events: Are you ok for me to commit these without the change to Str.cpp? That change made it handle silly (negative) parameters better, but at the cost of altering its output when given an empty string to match. No objection to the error handling, but I don't want to have to comb through the code base looking for situations where it might get passed an empty string and the code will end up doing something different when it gets -1 instead of the search start position as a return value.
  • Targets:thanks for the tracker issue & patch. I'll take this one too, but after I've cleared my workstack a bit because I want to look into whether I can implement some of the other improvements we discussed via PM. I'll start a feature proposal discussion for that, but after I've got rid of some existing tracker issues. I have 16 trackers open right now, 14 of them partially implemented across 8 different copies of the darkmod_src repo, so I'm in danger of losing track and need to get some closed down :wacko:
  • Language patch: we're stuck here. I can't commit a patch that crashes TDM when I test it, and I don't want to stop to learn all about il8n right now, and I don't have or use perl so I can't try the script fix you suggested. Can anyone else help with this?


#32 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 13 September 2014 - 12:24 PM

Quick update on the various strands:

  • Linux compilation fixes: I've committed fix #2 under the same tracker as #1. I can see how this typo happened. VS2010 insists in displaying the /ai/ folder as /AI/. I tried changing it under project properties, which works in vs2013 but not vs2010, which immediatelty reverts to AI. Hence developers are likely to make mistakes when including it. The other folders seem to be displayed with correct case, so I don't know why AI is wrong.

Thank you. And no worries, these issues are easily fixed. Might be "autocorrect" (for whatever reasons)

  • String handling script events: Are you ok for me to commit these without the change to Str.cpp? That change made it handle silly (negative) parameters better, but at the cost of altering its output when given an empty string to match. No objection to the error handling, but I don't want to have to comb through the code base looking for situations where it might get passed an empty string and the code will end up doing something different when it gets -1 instead of the search start position as a return value.


Good catch, sure, apply it without. I'll come up with a better patch for the idStr part (or leave it as it is). (Edit: It wasn't supposed to change handling of empty strings. I long have had the idea to have a testsuite, that can be run inside the game to test the various functions. Maybe it is finally time to bite the bullet and write a testsuite that tests the most basic scripting interface functions and can be used to check against side-effects of patches. Manual testing will never really find all changes with the complecity TDM has reached. End of edit)

  • Targets:thanks for the tracker issue & patch. I'll take this one too, but after I've cleared my workstack a bit because I want to look into whether I can implement some of the other improvements we discussed via PM. I'll start a feature proposal discussion for that, but after I've got rid of some existing tracker issues. I have 16 trackers open right now, 14 of them partially implemented across 8 different copies of the darkmod_src repo, so I'm in danger of losing track and need to get some closed down :wacko:


No worries. The most important part is that these patches go in, as they improve performance of trigger_touch a lot. The other improvements (like being able to bind a trigger_touch to an other entity) are less important.

  • Language patch: we're stuck here. I can't commit a patch that crashes TDM when I test it, and I don't want to stop to learn all about il8n right now, and I don't have or use perl so I can't try the script fix you suggested. Can anyone else help with this?


For windows, you can install Perl from http://strawberryperl.com/ - you don't actually know Perl to run the script.

I could also run the script for you, but I don't actually have access to the TDM repo, so I no longer can access my own work (talk about irony...)

If you give me a copy of the Perl script, I can modify it for you and also run it and send the output.

Edited by Tels, 13 September 2014 - 12:26 PM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#33 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 15 September 2014 - 07:08 AM

I've attached a testmap and a patch to fix http://bugs.thedarkm...iew.php?id=2897 - although there are two open questions - what is the body id about,and should it be exposed to the script interface?

In the testmap, if you frob various things, you will see they will go flying due to the impulse being applied - the console outputs info, too. The pear does oddly not work, I guess it somehow does not get "detached" from the grabber by "holdEntity($null_entity)" - this might be another bug.
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#34 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3657 posts

Posted 15 September 2014 - 01:04 PM

String-manipulating script events committed.

what is the body id about,and should it be exposed to the script interface?

It's for articulated figures -- it says which of the rigid bodies in the collection gets the push. So yes it should be part of the interface. I'll a note to the generated wiki text when I get to it.

In the testmap, if you frob various things, you will see they will go flying due to the impulse being applied


That sounds like more fun than my test maps. You've got me idly wondering whether we could somehow implement thrown objects knocking off guards' helmets :) Not stealthy, but satisfying.

EDIT: fixed link, was pointing to wrong bug tracker.

Edited by SteveL, 15 September 2014 - 02:53 PM.


#35 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 15 September 2014 - 01:47 PM

String-manipulating script events committed.


Wonderful!

It's for articulated figures -- it says which of the rigid bodies in the collection gets the push. So yes it should be part of the interface. I'll a note to the generated wiki text when I get to it.


Ah, I had thought it is for rigi body collections (like bound children). Adding a little note to the function parameters would be good, too.

Hm, looking at ApplyImpulse(), it is also assumed the entity that applies the impulse is passed in. So that might need tobe part of the interface, too. So the script can say who or what applied the impulse (and the receiver can selectively ignore it).

Better patch coming in a minute.

That sounds like more fun than my test maps. You've got me idly wondering whether we could somehow implement thrown objects knocking off guards' helmets :) Not stealthy, but satisfying.


:D

The "bind children break apart when too much force is applied" was never finished (mea culpa!), but I long had the idea that you could break of chair legs by throwing them around. :ph34r:
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#36 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 15 September 2014 - 02:59 PM

Patch and testmap for ApplyImpulse() have been updated.
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#37 gnartsch

gnartsch

    Member

  • Member
  • PipPip
  • 473 posts

Posted 19 September 2014 - 01:56 PM

Hi Tels,
would you happen to be able to recall what the fix for this issue was ?
http://bugs.thedarkm...iew.php?id=3307
The problem is there again, ... or the fix was overwritten by someone before it had a chance to take effect inTDM 2.0.
Don't know.
At any rate it is still around.
http://forums.thedar...when-completed/

#38 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 20 September 2014 - 03:44 AM

I don't think the problem in the bug report is the same as you are seeing. In the bug, the fix was to change a copy&paste error in the GUI. The effect was that one of the tickmarks was drawn in the wrong place. I no longer have access to the TDM main trunk, so I cannot check out my own fix....

Edit: Posted a possible fix in the other thread: http://forums.thedar...when-completed/

Anyway, there is/was another bug in where missions do not get properly checked off if the mission description or name didn't match exactly. AFAIR it had something to do with "FMName" vs. "fmname" in the server database.

From the screenshots in the other hread, tho, it looks like the checkmarks appear on the wrong page. (E.g. if you scroll down one page, the checkmarks appear for page 1 instead of 2). So that would be a different bug, unless it only affects the 11th line.

However, as I said, I no longer have access to the main mod, so I can only do limited testing, and not really fix it, anyway. Sorry.

Edited by Tels, 20 September 2014 - 04:02 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#39 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 21 September 2014 - 07:23 AM

Is there any way to not have a dynamic ambient light fade? Looking through the code in tdm_location_settings.script, it seems it will always fade the main ambient light (unless the light does not exist).

I'm asking because I want to control the ambient light myself (for special effects) and the code in the script interferes. Setting for instance "ambient_light" "0 0 0" and "ambient_light_dynamic" "0 0 0" on the location settings entity will always keep the ambient light in each location and at all the times at "0 0 0", so if you set it to some other value, the light will fade to black again.

One workaround would be to add another main ambient light to the map and manipulate this. But I'm not sure if "ambient_world" isn't rendered, anyway, even if it is black and/or has a radius of 0.

I can send a patch for adding a spawnarg to the location_settings entity like "no_ambient_light_fade" and track the issue, if nobody has any other idea.

Edit: Another, slightly costly method would be to copy the existing script object and take out the ambient light fade (but keep the ambient music fade). But I'd rather just disable the ambient light part with a spawnarg instead.

Edit #2: Another workaround would be set "ambient_light" on the location entity for the current location, then call

// player entered new location, so update ambient base color and fade time/delay
void speaker_zone_ambient::updateAmbientLight( entity locEnt )
  {

and this will read out the new value and fade towards it.

Edit #2: Yes, the latter does actually work. However, in my case, the ambient light was not fading because my code had a mistake and did not include my "fading" script. After including it again, the ambient light changes, even without the code above. That, however, prompted me to investigate how the ambient light ever could appear, because I don't actually set "ambient_light" on the location entities or the main location settings entity. I believe it is due to this bug: http://bugs.thedarkm...iew.php?id=2901

Basically, what happens is that my script attaches a small fog/blend light to the player. And since these lights are counted and are very close to the player, the ambient light gets brightened by them due to the "dynamic cap" part. Taking the player lantern out will also cause the ambient light to raise. (It might even be that the main ambient light gets added into the calculation, too.)

So the bug above causes accidentily some faint ambient light. If it was fixed, I'd have the problem to get it back without the ambient fading script to interfere. So in closing, I'd like to still fix issue 2901 (tone down the player lantern, do not count fog/blend lights in the calculation of LightInPVS()) and also add a spawnarg to the ambient light script to prevent it from fading the main ambient light if one doesn't want it.

Does somebody object to these changes?

Edited by Tels, 22 September 2014 - 02:45 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#40 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 22 September 2014 - 02:39 AM

Steve, thak you for applying the fix for http://bugs.thedarkm...iew.php?id=2897 :)

One question I still have regarding the patch, or rather, documentation: The impulse is applied to a point, and in my testmap I used the origin of the entity. I guess this is a good point/start? Or should the mapper somehow chose a point on the surface of the entity?


Could I devert your attention now to the trigger_touch patches? The performance fix there is crucial for any map that uses more than 5 of them :ph34r:

I'll work on the language patch today, so we can get that out of the door as well.
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#41 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3657 posts

Posted 22 September 2014 - 02:55 AM

Re the ambient light setup: what happens you use no ambient light related spawnargs at all on any of your location entities? Does the location system still fiddle with it? I did take a quick look yesterday and couldn't see that it was using any default for the ambient_light spawnarg.

#42 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 22 September 2014 - 03:22 AM

Re the ambient light setup: what happens you use no ambient light related spawnargs at all on any of your location entities? Does the location system still fiddle with it? I did take a quick look yesterday and couldn't see that it was using any default for the ambient_light spawnarg.


It uses '0 0 0' a default:

	entity ambientWorld = sys.getMainAmbientLight();
	if ( ambientWorld != $null_entity )
	{
		// the default color (greebo: use the _color spawnarg, the entity colour might have changed already)
		m_al_default_color = ambientWorld.getVectorKey("_color");
		sys.println("Color read from main ambient light '" + ambientWorld.getName() + "': " + m_al_default_color);
	}
	else
	{
		m_al_default_color = '0 0 0';
		sys.println("No main ambient light, so color assigned: " + m_al_default_color);
	}

However, it will also read out

		// Tels: query here the ambient_light_dynamic factor:
		vector dyn = locEnt.getVectorKey("ambient_light_dynamic");

		if (dyn_x != 0 || dyn_y != 0 || dyn_z != 0)

and if that is != 0, it will compute the light in the PVS and add it to the main ambient light. If you change afterwards the main ambient light, the script does not know about this and fades it back to what it think it should be:

		    /* Tels: Update the new ambient base color and fade times/delays */
		    updateAmbientLight( locEnt );

It doesn't have any way to not do that, at least as far as I can see.

Edited by Tels, 22 September 2014 - 03:23 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#43 SteveL

SteveL

    Hero Coder

  • Active Developer
  • PipPipPipPip
  • 3657 posts

Posted 22 September 2014 - 04:25 AM

So with no ambient_light spawnargs at all, the location scripts will use your starting ambient light setting and always fade to that. Do the scripts read the spawnarg for each location only once? Just wondering whether you could get round it by changing the key on the location entity to what you want the color to be.

One question I still have regarding the patch, or rather, documentation: The impulse is applied to a point, and in my testmap I used the origin of the entity. I guess this is a good point/start? Or should the mapper somehow chose a point on the surface of the entity?

Doing it from the origin probably isn't the most useful way. I noticed in the test map that the objects fly off in a random direction, because the impulse applied is straight up and from the origin, which is usually the centre of the object's base. If I were using this in a real script I'd do a trace first and apply the impulse to the contact point, or maybe apply the impulse to the centre of the bounding box, which would be cheaper to calculate but should give the right directional result too, if it works.

#44 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 22 September 2014 - 06:02 AM

So with no ambient_light spawnargs at all, the location scripts will use your starting ambient light setting and always fade to that. Do the scripts read the spawnarg for each location only once? Just wondering whether you could get round it by changing the key on the location entity to what you want the color to be.


Yes, that is basically correct. The location script will assume '0 0 0' if nothing is set, and they will read the new value once you enter a new location and then always "fade" to that value.

Note that this is different from the "dynamic" part of the ambient light. The scriptobject on the location settings entity will also fade the ambient to the new color if the dynamic part is zero and the light doesn't have changed. It will simply set the new value every time it runs, even if this is the old value.

So if you manually modify the ambient light in a location, the script will simply overwrite it again.

I have tracked the issue here: http://bugs.thedarkm...iew.php?id=3859

Attached to the tracker are also a testmap and a patch that implements a spawnarg that can stop the fading for each location.

no_ambient_light_fade is set to "1" globally, and one location (red) sets it back to "0".

The map has three parts:

* starting area. green ambient. (The new script does not detect that it should fade the new value once when it starts. I found it difficult to fix, and it is not really nec. for this demo, as when you set "no_ambient_light_fade" to 1, you are supposed to control the ambient light yourself. The starting area just shows the ambient is left alone.

If you return to the starting area, the ambient light will be faded once to green, then left alone.

* red area (to the right). "no_ambient_light_fade" is "0", so the dynamic ambient fading will start here. (This area also shows that the player lantern overpowers the ambient - go to the far corner, look back and turn your lantern on and see the entire room turn reddish).

* the area straight ahead. It has "no_ambient_light_fade" "-1" and thus uses the global value "1". which means no fading. There is a script attached which messes with the ambient in this area - you may like what it does :)
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#45 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 22 September 2014 - 07:27 AM

I've attached another patch to tdm_location_settings.script to the bug http://bugs.thedarkm...iew.php?id=3694 as well as a new testmap (and the final version of this file). This one is on top of the previous and fixes 3694 as well (fade ambient to the default value when entering invalid zones). It also fixes the issue when starting in a "no_ambient_light_fade" zone (the start area turns green on start).
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#46 nbohr1more

nbohr1more

    Darkmod PR, Wordsmith

  • Development Role
  • PipPipPipPipPip
  • 7333 posts

Posted 23 September 2014 - 07:56 PM

Sorry to squeak from the peanut gallery here but shouldn't most of this script (if not all) be moved to the game code
where it'll have a tad less performance impact?

I guess it's easier to adjust in script form but some of this stuff is pretty set-in-stone now and wouldn't
be stuff most players would tinker or fuss with. Same with a number of these base scripts
(tdm_lights, tdm_light_holder, tdm_readables). Especially since the lights in TDM are already so customized
with think code, why do it half in SDK and half in script?

This would also move it into the public SVN...

Edited by nbohr1more, 23 September 2014 - 08:49 PM.

  • New Horizon likes this
Please visit TDM's IndieDB site and help promote the mod:

http://www.indiedb.c...ds/the-dark-mod

(Yeah, shameless promotion... but traffic is traffic folks...)

#47 New Horizon

New Horizon

    Mod hero

  • Active Developer
  • PipPipPipPipPip
  • 13676 posts

Posted 24 September 2014 - 07:10 AM

Sorry to squeak from the peanut gallery here but shouldn't most of this script (if not all) be moved to the game code
where it'll have a tad less performance impact?


Good idea.

#48 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 26 September 2014 - 10:45 AM

Sorry to squeak from the peanut gallery here but shouldn't most of this script (if not all) be moved to the game code
where it'll have a tad less performance impact?

I guess it's easier to adjust in script form but some of this stuff is pretty set-in-stone now and wouldn't
be stuff most players would tinker or fuss with. Same with a number of these base scripts
(tdm_lights, tdm_light_holder, tdm_readables). Especially since the lights in TDM are already so customized
with think code, why do it half in SDK and half in script?


Theoretically, yes it could happen, as most of the mechanics are now set in stone.

But the ambient light script runs only 5 times per second and it does only a handful of script commands, which are pretty fast, too. You would not gain any relevant performance impact. The most complicated step, finding out how much light is in the PVS according to distance rules, is a single script call, and runs in C++, anyway.

Edit #2: The loction script does only do more work if the location changes. 99.9% of the time the location doesn't change as the player is standing still or moving in the same location. Edit 2 end.

And it would be quite some work to port it over, test it and so on.

As for the other script objects, haviung them in script object makes it much easier to modify, and to inherit from these. If you move them to game code, the mapper will lose the control, and you gain no noticable performance - the light holder script object are only active when light's status changes, e.g. not contionuisly.

Edit 3: Script calls like LightsOn() cannot be moved to C++ code (I'm nearly sure), because the script call happens on the script object, which must exist for that to work. So you couldn't even move the entire script object to C++, so you would still need a hybrid solution, otherwise maps break. That would be even more complicated than the single file script object we have right now. End of Edit 3.

So you would lose the flexibility, do a lot of work for that, and gain almost nothing in return.

I think there are more important things to do than to optimize non-issues like that. For instance more translation work, fixing real bugs, adding new features and so on. It's not my decision, tho. If you find someone, great, I won't mind :)

Edit: For instance, optimizing the script language in general would bring a much greater benefit, not only to TDM. Some of these patches could be even ported to other D3 engines. End of edit.

This would also move it into the public SVN...


The public SVN is still read-only, tho.

I think a lot of things like entity defs, scripts and so on should be public. Maybe even the entire repository. OTOH, it is understandable that a "stable" repo is important, and the team needs to work without people distracting them. But then, come release day, the modifications are released to them public, anyway.

Edited by Tels, 26 September 2014 - 10:54 AM.

"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax

#49 nbohr1more

nbohr1more

    Darkmod PR, Wordsmith

  • Development Role
  • PipPipPipPipPip
  • 7333 posts

Posted 26 September 2014 - 02:29 PM

Yeah, if anyone ever gets LLVM or some other JIT to replace the script interpreter, then all these scripts would run as fast as native code anyway. Thanks for the clarifications.
Please visit TDM's IndieDB site and help promote the mod:

http://www.indiedb.c...ds/the-dark-mod

(Yeah, shameless promotion... but traffic is traffic folks...)

#50 Tels

Tels

    Mod hero

  • Member
  • PipPipPipPipPip
  • 15024 posts

Posted 27 September 2014 - 04:48 AM

Here is a new version of the language patch (updating German, adding Swedish) including the nec. changes on the gen_lang.pl script.

http://swift-mazes.c..._2014-09-27.zip

I've added a few more missing German translations at the same time.

TODO: The new string #9000 should not block needlessly the 9000er range, instead if should be moved to one of the blocks like 25xx. I can do the change, but whever this string is used, the usage needs to be changed, too, and I cannot do this.
"The reasonable man adapts himself to the world; the unreasonable one persists in trying to adapt the world to himself. Therefore, all progress depends on the unreasonable man." -- George Bernard Shaw (1856 - 1950)

"Remember: If the game lets you do it, it's not cheating." -- Xarax




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users