(V80c) Issues using 'IntObject' with 'Abs' & 'Sgn'

Monkey Forums/Monkey Bug Reports/(V80c) Issues using 'IntObject' with 'Abs' & 'Sgn'

ImmutableOctet(SKNG)

(Posted 2014) [#1]

So, I was cleaning up some of my code, and I noticed that 'reflection' decided it wanted to break a few things. At first I didn't understand it, but then it dawned on me, the 'IntObject' class for some ungodly reason (Literally, the error was on line 666 of my 'vector' module) has a 'ToFloat' implementation.

This makes 'IntObjects' unusable without manually calling 'ToInt'. And since this is a generic/template class, I can't do that. I would just cast, but it's not like I have a way of telling what type it's based on.

The issue here is that 'Trans' doesn't look deep enough. As an example, try removing 'ToFloat' for 'IntObject'. The result causes an error when doing this: "Float(IO)"; where 'IO' is an 'IntObject'. And in my case, I can't even use explicit casting like that. Now, if 'Trans' checked if it had something to auto-convert with, this would just call 'ToInt' as expected.

You see, you can make the argument all you want about being explicit with "ToBlah", but this is broken. Not only is it an issue for commands like 'Abs' which take either a float or an integer, but this can also break generic classes in the process.

This leaves me at a dead end which can only be solved one of four ways:

1) You add a keyword for preferences with "ToBlah". (A final say on what should be done - Not a bad idea)
2) You remove the 'ToFloat' overload, and we push this off even longer.
3) You have 'Trans' look at the type for further conversion options after the fact (A VERY GOOD IDEA)
4) One of us makes awful versions of these commands that just call the normal versions (PLEASE DON'T DO THIS UNLESS IT'S TEMPORARY. I did this to my own code, and it was not fun. But Monkey itself relying on such hacks would get even more messy, not to mention go against the idea of "ToBlah")

A bit of elaboration on the third option: Going this route will allow you to remove those odd "ToBlah" instances, and it'll fix my issue.

And with a small bit of eventual work, it'll even fix that problem we've talked about previously (Conversion to super-classes being odd with "ToBlah"); though, that's a bit of a different subject.

The same goes for 'ToInt' in 'FloatObject' by the way. And no, don't remove auto-conversion for objects, there's no reason to; I know you've been on the fence about that. I use it on a regular basis, and I think it's a useful feature. You'd give me (And many others) far more headaches by removing it.

This issue also broke several parts of my code. (Which I was actually able to fix on my end. Which is at least acceptable, since there are ways of working around such issues with my own code)

Now I'm bottlenecked by this sort of bug again, but now it's undoubtedly an issue with Monkey's 'boxes' module (Deeper technically, but that's the surface issue). If this doesn't get fixed, I'll have to make things even worse by creating private wrappers for default commands within my modules which use option 4. Or I could do it for the standard 'monkey' modules, but that's a ticking time-bomb.

ImmutableOctet(SKNG)

(Posted 2014) [#2]

Just as a heads up, I'm attempting to patch this for my own modules, but that's going to take a while. Anyone who's using my modules will have to wait for everything to be completed.

And Mark, I've gone with an alternate version of my 4th suggestion. I need my code to work now, but I still expect at least something to be done in the future. I'm attempting to future-proof as much as possible, so some code could be completely ignored when/if this is changed. If you're currently working on a fix for this, please continue.

P.S. I made a module which will do some of the work for me if I need it. Though, I'm trying something a bit more intrusive, which will actually allow other options for future modification of my code.

marksibly

(Posted 2014) [#3]

Not enough info...can you provide some sample code? Is it only reflection you're having a problem with? etc. Some idea of what you're trying to achieve would also be nice - perhaps monkey needs some kind of variant system (which is what boxes kind of do in a half-assed way)?

The box stuff is really only in there because of the way monkey generics used to work - if I'd done them the right way in the first place, boxing probably wouldn't be in there at all. But don't worry, I wont rip them out!

The 'odd' ToInt/ToFloat stuff is in there mainly to mimic the implicit conversions allowed by real ints/floats etc. Back in the 'old generics' days, this allowed you to treat box objects pretty much as if they were primitives.

ImmutableOctet(SKNG)

(Posted 2014) [#4]

The issue's pretty easily found when importing 'reflection' and setting the filter to "*" / adding 'monkey.math' to the filter. EDIT: This also happens without reflection imported.

The thing is, my issue was also with generic/template classes. But I think that makes more sense than the 'IntObject' thing.

Here's an example of using 'IntObject' with the 'Abs' command:

Strict

Public

' Preprocessor related:
#REFLECTION_FILTER = "*"

' Imports:
Import reflection

' Functions:
Function Main:Int()
	Local IO:IntObject = -100
	
	Print(Abs(IO))
	
	' Return the default response.
	Return 0
End

Here's a more realistic example of my specific issue (Though, I'm not sure how I feel about this):

Strict

Public

' Preprocessor related:
#REFLECTION_FILTER = "*"

' Imports:
Import reflection
Import monkey.boxes

' Functions:
Function Main:Int()
	' Local variable(s):
	Local IO:= New IntObject(255)
	
	' Output to the console:
	Print(Test<IntObject>.AsInt(IO))
	
	' Return the default response.
	Return 0
End

' Classes:
Class Test<T>
	' Functions:
	Function AsInt:Int(O:T)
		Return O.ToInt()
	End
	
	' This is the issue.
	Function AsBool:Bool(O:T)
		Return O.AsBool()
	End
End

The thing is, the second example logically should error with reflection, as it generates an invalid function. But at the same time, this means reflection is completely out of the question for some of my modules (My old 'screen' module used vectors made from 'IntObjects').

My exact issue with my 'vector' module has to do with commands like 'Abs', 'Min', 'Max', and 'Clamp'. To the same effect, simple adding via "+=" breaks it, however that makes a bit more sense (Though it would be nice if it did work).

It wouldn't fix all of my problems (Those I can fix, it's just annoying), but I think the best idea right now is a preference keyword for "ToBlah". Something which will gear 'Trans' in the right direction about these situations. I don't think that'll cause any issues that weren't already accounted for.

Just for the sake of listing everything that causes an error in my 'vector' module alone:
* +=; This one makes at least some sense, though having the shorthand would be nice regardless. (Only an issue with actual 'IntObjects', it works with integers just fine) - Also see here, here, and here. That's why being able to use the "+=" shorthand would be a useful addition.
* -IO; where 'IO' is an 'IntObject'.

Uses of commands not working due to having both 'ToInt' and 'ToFloat' implemented for 'IntObject' (And likely 'FloatObject' by extension):
* Abs: 01, 02.
* Min: 01, 02, 03.
* Max: 01, 02, 03.
* Clamp: 01, 02, 03.

This apparently has been a long-running thing with my 'screen' module. I just never noticed this, since I tend to stay away from reflection for the sake of simplifying code (My render/draw routines with "C++ Tool" for example). I'd just like to add that Monkey's "Compile what's used" is really useful, even if it can cause some issues for newer users.

EDIT: By the way, I wouldn't recommend using my 'vector' module to test this. Not because it won't error, but because this was before I fixed some issues relating to these. By the way, what was the reasoning for not allowing globals in interfaces? I assume this is a language issue with some targets.

ImmutableOctet(SKNG)

(Posted 2014) [#5]

With all of that in mind, my solution was to create another layer to wrap existing commands like 'Abs'. The problem is (Though I could optimize things in my case for 'IntObjects') it's another function call for normal integers and floats. Assuming optimization fixes this, it's not a big deal, but I still end up having to write wrappers for everything. Given, I'll likely go with this approach anyway, but that shouldn't apply to all situations. I'm honestly for a preference keyword when declaring a function.

Maybe something like:

Method ToInt:Int() Preferred ' Or something with a better ring to it?
	Return value
End

If implemented into 'Trans', this would make calling 'Abs' unambiguous in my case (Which is what you'd expect for something called 'IntObject'). Basically, 'Trans' would prefer 'ToInt' over 'ToFloat'.

EDIT: Also, the "Preferred" name does have some ring to it. Think of writing this: "Preferred Abstract Property" - That looks pretty nice to me.

And as for the whole "reflection with generic classes" thing, I personally would like the usual way of error checking (Check as used) for such situations, but hey, a guy can dream, right? Also note that my 'util' module's 'GenericUtilities' class will eventually have issues the way reflection currently checks functions. This effect is already seen in my 'boxutil' module, here.