Bug 3997 - Assign operators for floats and integers don't exist in float.inc
Assign operators for floats and integers don't exist in float.inc
Status: RESOLVED FIXED
Product: AMX Mod X
Classification: Unclassified
Component: Core
trunk
PC Windows
: P3 trivial
Assigned To: amxmodx-bugs@alliedmods.net
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-08 08:18 PDT by 5now
Modified: 2014-04-09 11:44 PDT (History)
5 users (show)

See Also:


Attachments
assign operators for integers and floats - .inc file (1.11 KB, text/plain)
2009-09-08 08:18 PDT, 5now
no flags Details
Proposed patch 1 (72 bytes, patch)
2009-10-22 07:25 PDT, Lev2001
no flags Details | Diff | Splinter Review
Proposed patch 2 (115 bytes, patch)
2009-10-22 07:26 PDT, Lev2001
no flags Details | Diff | Splinter Review

Description 5now 2009-09-08 08:18:51 PDT
Created attachment 1701 [details]
assign operators for integers and floats - .inc file

Assign operators for floats and integers don't exist in float.inc. 
The thing you've to do currently is:
new inum = floatround( fnum );
new fnum = float( inum );

I don't know whether there's some reason why they weren't added when the other user defined operators were, somehow I wrote them up myself and they seemed working fine. As the attachment there's an include file which includes the plugin I tested the inc with. I added them to the one file, since weren't able to submit two.
Comment 1 David Anderson [:dvander] 2009-09-08 11:16:00 PDT
> new fnum = float( inum );

I hope you meant |new Float:fnum| there.

I'm not sure I like the idea of the Float-to-int assignment operator. Making lossy conversion implicit sounds like a great way to mask subtle bugs.

int-to-Float is a good idea though.

Please attach source code changes as diffs or patches - if you need help let me know.
Comment 2 5now 2009-09-08 12:50:07 PDT
Yes, I meant there Float: tag, obviously.

float.inc, the other stock is only a place mark:

stock Float:operator-(Float:oper)
    return oper^Float:cellmin; /* IEEE values are sign/magnitude */

->

stock Float:operator-(Float:oper)
    return oper^Float:cellmin; /* IEEE values are sign/magnitude */

stock Float:operator=(oper1)
	return float(oper1);
Comment 3 5now 2009-09-08 12:53:31 PDT
I wasn't paying enough attention to the variable name style either. Here's the correct version:

stock Float:operator-(Float:oper)
    return oper^Float:cellmin; /* IEEE values are sign/magnitude */

->

stock Float:operator-(Float:oper)
    return oper^Float:cellmin; /* IEEE values are sign/magnitude */

stock Float:operator=(oper)
    return float(oper);
Comment 4 Lev2001 2009-10-22 07:23:10 PDT
Let me help small.
I attached 2 diff files:
1 one only adding operator= for int to float conversion.
2 one adding operator= for int to float conversion and also forbid operator= for float to int conversion (i.e. instead of "tag mismatch" and successful compilation you will get error in compilation when try to assign float to int).
I think that second one is better because I didn't see a sense in assigning float to int so that target receives value that has little or nothing to initial value.
Comment 5 Lev2001 2009-10-22 07:25:40 PDT
Created attachment 1759 [details] [review]
Proposed patch 1

only adding operator= for int to float conversion
Comment 6 Lev2001 2009-10-22 07:26:41 PDT
Created attachment 1760 [details] [review]
Proposed patch 2

adding operator= for int to float conversion and also forbid operator=
for float to int conversion
Comment 7 David Anderson [:dvander] 2013-02-13 01:06:08 PST
From a language design perspective, this is a great idea and makes a ton of sense.

There's an argument that Pawn's weird "type" system has become really embedded in the community - so much so that, even in SourcePawn, to date we have not seen anyone confused by integer to float assignment. Having the new rule will also be inconsistent with:

  native X(Float:y);

  X(5);

Those aren't really good arguments, but for 1.8.2 chemspill I'm going to classify this as a feature rather than a bug, and sit on it for now.
Comment 8 Vincent Herbet [:Arkshine] 2013-08-23 06:26:55 PDT
Well, yes, not really convincing arguments. I've tried to use it in several situations and it would makes sense to add it as it's always an implicit conversion int -> float and expliciting float() appears unnecessary most of time.
Float: being always here, I don't think people could get confused.

That's said, I'm concerned about something.
If you do :

> new Float:value1 = 15.0;
> new Float:value2 = 15;

First, float() won't be called.
Second, float() will be called.

I'm not sure about this, even if documented, people will tend most likely to use direct value without ".", result calling an unnecessary native.

At the same time, difference is really trivial so it would be fine.
Well, I think I'm going to add this unless someone finds better arguments to not add it!
Comment 9 David Anderson [:dvander] 2013-08-23 12:05:09 PDT
int-to-float is fine, as per comment #1 and comment #7. float-to-int should not be implicit.
Comment 10 AM Bugzilla Bot 2013-08-24 14:49:48 PDT
Pushed changeset: http://hg.alliedmods.net/amxmodx-central/rev/2bc36c43b15f
Changelog:
	Add assign operator for floats to allow implicit int-to-float conversion (bug 3997, r=dvander)
Comment 11 Vincent Herbet [:Arkshine] 2013-09-10 00:08:20 PDT
This adds warning/error on compilation depending the situation whereas before not. The fault is because of the buggy compiler which doesn't retrieve properly the tag on left expression. It happens with the use of enum and this patch just highlights such buggy compiler behavior. I think it has been fixed on newer version. Anyway for now, until a proper solution is found, reverting this.
Comment 12 Erin Baker [:asherkin] 2013-10-05 09:26:08 PDT
What was the warning, and do you have some code samples?
Comment 13 AM Bugzilla Bot 2014-02-08 23:53:22 PST
Commit pushed to master at https://github.com/alliedmodders/amxmodx

https://github.com/alliedmodders/amxmodx/commit/d9fb9bac09b69b81819b97cf2ac23e7e20a49f27
Add assign operator for floats to allow implicit int-to-float conversion (bug 3997, r=dvander)


Former-commit-id: 04b6a21f239bb9332a1410d22c8c3c45510a28b2

Note You need to log in before you can comment on or make changes to this bug.