Login
User Name:

Password:



Register
Forgot your password?
Vote for Us!
Couple bugs
Dec 12, 2017, 5:42 pm
By Remcon
Bug in disarm( )
Nov 12, 2017, 6:54 pm
By GatewaySysop
Bug in will_fall( )
Oct 23, 2017, 1:35 am
By GatewaySysop
Bug in do_zap( ), do_brandish( )
Oct 18, 2017, 1:52 pm
By GatewaySysop
Bug in get_exp_worth( )
Oct 10, 2017, 1:26 am
By GatewaySysop
LOP 1.45
Author: Remcon
Submitted by: Remcon
LOP Heroes Edition
Author: Vladaar
Submitted by: Vladaar
Heroes sound extras
Author: Vladaar
Submitted by: Vladaar
6Dragons 4.3
Author: Vladaar
Submitted by: Vladaar
Memwatch
Author: Johan Lindh
Submitted by: Vladaar
Users Online
CommonCrawl, Yandex, Sogou, Bing, DotBot, Google

Members: 0
Guests: 8
Stats
Files
Topics
Posts
Members
Newest Member
477
3,705
19,232
608
LAntorcha
Today's Birthdays
There are no member birthdays today.
Related Links
» SmaugMuds.org » General » Coding » UMAX-UMIN-URANGE-etc..
Forum Rules | Mark all | Recent Posts

UMAX-UMIN-URANGE-etc..
< Newer Topic :: Older Topic > ...problems...

Pages:<< prev 1 next >>
Post is unread #1 Mar 23, 2005, 5:55 am   Last edited Sep 24, 2009, 3:26 am by Samson
Go to the top of the page
Go to the bottom of the page

Matteo2303
Apprentice
GroupMembers
Posts57
JoinedAug 25, 2003

I think that the UMAX (umin, etc..) macro isn't a good thing in some case.

Example: ------------------------
value = UMAX( 1, 5 - dice(1, 10) );
---------------------------------

Value could be < 1 because the macro replace the "dice" function and reroll (I think).

So, in smaug rd_parse function, we find:

case '{':
total = UMIN( total, rd_parse( ch, level, sexp[1] ) );
break;
case '}':
total = UMAX( total, rd_parse( ch, level, sexp[1] ) );
break;

...but the rd_parse( ch, level, sexp[1] ) could be called by an expression like "5-Ld10", or not?

The same problems there is the code about other things like here (spell_obj_inv):
water = UMIN( ( skill->dice ? dice_parse( ch, level, skill->dice ) : level ) * ( weath->precip >= 0 ? 2 : 1 ), obj->value[0] - obj->value[1] );

...or here (spell_create_mob):
mob->level = UMIN( lvl, skill->dice ? dice_parse( ch, level, skill->dice ) : mob->level );

...or here:
mob->max_hit = UMAX( URANGE( mob->max_hit / 4,
( mob->max_hit * corpse->value[3] ) / 100, ch->level * dice( 20, 10 ) ), 1 );

Is it a bug, or not?
Did you think that is better make a new UMAX (umin, etc..) macro or FIX storing the variant value before call the macro?

Bye bye
matteo
       
Post is unread #2 Apr 3, 2005, 11:41 am
Go to the top of the page
Go to the bottom of the page

Xorith
The Null Value
GroupAFKMud Team
Posts254
JoinedFeb 23, 2003

I mean no disrespect, but I don't quite follow what your meaning is in this post.

Are you reporting a bug, or are you speculating if something is working as intended?
       
Post is unread #3 Apr 3, 2005, 12:59 pm   Last edited Nov 24, 2007, 1:52 pm by Samson
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

UMAX is fairly simple. It takes the largest of the 2 values you're comparing, and simply returns that to whatever you're doing.

value = UMAX( 1, 5 - dice(1, 10) );


In this case, if 1 is the larger of the two, then it gets returned. If 5-dice(1,10) is larger, then it gets returned. This isn't a bug, this is expected behavior of this macro.

rd_parse is a whole other thing entirely and has been a sore spot for ages. Nobody, not even those who claim to, have actually fixed it. It would almost be worth paying money to get someone to fix the dice formula code.
       
Post is unread #4 Apr 4, 2005, 9:16 am
Go to the top of the page
Go to the bottom of the page

Matteo2303
Apprentice
GroupMembers
Posts57
JoinedAug 25, 2003

I'm with you Samson: it isn't a true bug. The only thing that I wanted show is that an expression like:

UMAX( 1, 5 - dice(1, 10) );

could return a value < 1, and when in the code there is an UMAX (or UMIN or URANGE) with a random value, is possible that the return_value isn't a value that we want.

Thank you
matteo
       
Post is unread #5 Nov 24, 2007, 1:53 pm
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

Thread necromancy alert:

UMAX( 1, 5 - dice(1, 10) );

could return a value < 1


That's impossible. For reasons explained 2 years ago. UMAX always returns the larger value, so it could never return less than 1 in this example usage.
       
Post is unread #6 Nov 26, 2007, 8:17 am
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

Actually, no, it could return a value less than one. Welcome to the world of macros...

#define UMAX(a, b) ((a) > (b) ? (a) : (b))

Note that a macro literally replaces its arguments with what you use as the macro parameters. So,

UMAX( 1, 5 - dice(1, 10) );

is really

((1) > (5 - dice(1, 10)) ? (1) : (5 - dice(1, 10));

You could have a first dice result less than four, such that 1 is greater than it and we return one. But, if 1 is less than the dice result, we return the expression (5 - dice(1, 10) -- a new evaluation of that expression. Hence, we could return anything in the range 4 to -10. The crux of the problem is that the result you are returning is not the result you compared.

This is why it would be better for UMAX to be a function than a macro. The argument expressions would only be evaluated once. The compiler would probably inline it anyhow so it wouldn't be less efficient (for the tiny, absolutely irrelevant (in the scale of things) gain in efficiency you'd get anyhow).

You might have noticed macros that say things like "this macro only evaluates its arguments once". Well, the UMAX macro evaluates each argument once to compare, and then the result argument again.
       
Post is unread #7 Nov 26, 2007, 12:46 pm   Last edited Nov 26, 2007, 12:47 pm by Samson
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

Oh. Now that you've explained it that way I see you're right. And UMAX would indeed be better treated as a function to prevent reevaluating function calls as arguments.

See, I knew this merger would be beneficial. That post sat on the AFKMud side for 2 years without a sufficient explanation that I thought clarifying would help :)
       
Post is unread #8 Nov 26, 2007, 1:02 pm
Go to the top of the page
Go to the bottom of the page

Remcon
Geomancer
GroupAdministrators
Posts1,858
JoinedJul 26, 2005

Well just to be sure I checked it out using UMAX and it was possible. It didn't take long to have the UMAX( 1, 5 - dice(1, 10) ) return a -5 value.
       
Post is unread #9 Jan 12, 2008, 8:22 pm   Last edited Jan 12, 2008, 9:08 pm by GatewaySysop
Go to the top of the page
Go to the bottom of the page

GatewaySysop
Conjurer
GroupMembers
Posts367
JoinedMar 7, 2005

Hmm.

I have noticed one oddity when using this fix, and that is a complaint from the compiler on this line in update.c, in the function reboot_check:

static const int timesize = UMIN( sizeof( times ) / sizeof( *times ), sizeof( tmsg ) / sizeof( *tmsg ) );


Any takers on how to get around the "initializer not constant" issue here?

       
Post is unread #10 Jan 12, 2008, 8:26 pm
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

I ran into the same thing compiling the fix into the SWR packages. Got lazy and just used a constant value. Since there were 8 elements in the array, I defined timesize = 8 and left it at that for now.

I meant to post on that to ask for suggestions, but forgot :P
       
Post is unread #11 Jan 12, 2008, 10:49 pm
Go to the top of the page
Go to the bottom of the page

Gatz
Apprentice
GroupMembers
Posts60
JoinedJul 25, 2005

Hrm, if I'm not that off in my thinking, I believe the error is caused because the compiling tries to evaluate all const values at compile time. Now that UMIN is not a function and not a macro, it can't do that. The solution I went with was not make timesize a static const, but a plain int. I dug what it was doing, which is basically figuring out how many elements are in tmsg and times and then picking the smallest. I doubt most people edit those a lot, so hard setting it probably just as good.

I dunno what else could be done to fix it, minus a large (read: less lazy) approach.
       
Post is unread #12 Jan 13, 2008, 12:21 am
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

An ugly, but acceptable, way to get around that would be to just do what the old macro did for cases where things need to be known at compile time. Either add the macro version in under another name, or write it fully expanded out.

static const int timesize = (sizeof( times ) / sizeof( *times )) < (sizeof( tmsg ) / sizeof( *tmsg ))
                                    ?  (sizeof( times ) / sizeof( *times ))
                                    :  (sizeof( tmsg ) / sizeof( *tmsg ));

       
Post is unread #13 Jan 13, 2008, 12:57 am
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

Gatz said:

Hrm, if I'm not that off in my thinking, I believe the error is caused because the compiling tries to evaluate all const values at compile time.

Actually it's the static const part here that's the problem -- not just the const. It's ok to have a non-static const value that is the result of a function.
       
Post is unread #14 Jan 13, 2008, 8:50 am
Go to the top of the page
Go to the bottom of the page

Samson
Black Hand
GroupAdministrators
Posts3,639
JoinedJan 1, 2002

Does this part of the code actually need to be static? And that raises another question. How do you know when something should be static?
       
Post is unread #15 Jan 13, 2008, 10:10 am
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

In C, the static keyword has two different :( functions.

If you're inside a function and declare a variable static, it causes it to be stored in the CODE segment of memory, which means it retains its value between calls. Otherwise, it is allocated from either the STACK or HEAP segments, which are freed once the function returns.

OTOH, if you declare a function OR a variable that's outside a function as static, it tells the compiler that those symbols are private and can only be accessed within the same object file (typically, the same source file).

Since these applications aren't generally subject to having user-supplied code dynamically loaded at run-time, I would say the static part isn't needed. The only (bad) way I could see it being needed is if there's another timesize variable in another module somewhere... in that case, the static qualifier would prevent the linker from throwing a fit over duplicate definitions... but that would be a very naughty thing to do.

Oh, while I'm here.... one unfortunate naming inconsistancy which this change brings up is that these really shouldn't be called umin, umax, and urange anymore. The "u" part is a holdover from "unsigned". However, now that these are functions and only accept signed integers, it's not really correct usage. Since min and max are often defined in the system math libraries, it might be proper to make these imin, imax and also make unsigned int versions.

I'm not trying to cause trouble here (honest!), but someday someone will try to call umax(2147483647, 2147483648) and wonder why they got 2147483647 back -- because 2147483648 becomes -1 in signed 32-bit integers.
       
Post is unread #16 Jan 13, 2008, 7:54 pm
Go to the top of the page
Go to the bottom of the page

David Haley
Sorcerer
GroupMembers
Posts903
JoinedJan 29, 2007

Quixadhal said:

If you're inside a function and declare a variable static, it causes it to be stored in the CODE segment of memory

Technically it goes into the data segment; the code segment is read-only. This is what allows the code segment to be shared across multiple instances of a program, for example. (And I'm sure you meant that only the stack is freed after the function exits, I'd just like to point out to avoid any confusion that exiting a function doesn't do anything to the heap directly.)

In any case, yes, you declare local variables static when you need them to keep their value across function calls. It's kind of like your own private global variable.
       
Post is unread #17 Jan 13, 2008, 8:39 pm
Go to the top of the page
Go to the bottom of the page

Quixadhal
Conjurer
GroupMembers
Posts398
JoinedMar 8, 2005

Good catch. Yeah, it's been a few years since I did assembly (on my trusty Amiga, Motorola 68000 CPU). The CODE segment there wasn't actually read-only, because that CPU didn't support protected memory, but writing self-modifying code was usually frowned upon. :) The 68010 and up did support protected memory segments, and I believe AmigaOS 2.0 used it if it was available.

       
Pages:<< prev 1 next >>