* (remove) doesn't work @ 2004-05-18 22:12 mskala 2004-05-18 22:26 ` mskala 2004-05-19 0:30 ` Eric McDonald 0 siblings, 2 replies; 7+ messages in thread From: mskala @ 2004-05-18 22:12 UTC (permalink / raw) To: xconq7 [-- Attachment #1: Type: TEXT/PLAIN, Size: 610 bytes --] As far as I can tell, the (remove) form simply returns its second argument instead of doing its job. Test case attached - this will fail regardless because of the lack of terrain types, but it'll also give an error about trying to match a length-7 list with another list that should also be length-7 but is in fact length-10. The only shipped game module using (remove) is tailhook.g. I haven't figured out whether (remove) is misbehaving in that file, but it definitely misbehaves in my test case. -- Matthew Skala mskala@ansuz.sooke.bc.ca Embrace and defend. http://ansuz.sooke.bc.ca/ [-- Attachment #2: Type: TEXT/PLAIN, Size: 857 bytes --] (game-module "remove" (title "Remove bug demo") (version "1.0") (blurb "Displays an error in the remove form.") ) (unit-type unit1 (image-name "adventurer")) (unit-type unit2 (image-name "adventurer")) (unit-type unit3 (image-name "adventurer")) (unit-type unit4 (image-name "adventurer")) (unit-type unit5 (image-name "adventurer")) (unit-type unit6 (image-name "adventurer")) (unit-type unit7 (image-name "adventurer")) (unit-type unit8 (image-name "adventurer")) (unit-type unit9 (image-name "adventurer")) (unit-type unit10 (image-name "adventurer")) (define nonspecialunits (remove (unit1 unit2 unit3) u*)) ;;;; Here's the error: the "remove" form should have set nonspecialunits to ;;;; a length-7 list (10 minus 3) but instead it will have set it to a ;;;; length-10 list. (add nonspecialunits hp-max (1 2 3 4 5 6 7)) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (remove) doesn't work 2004-05-18 22:12 (remove) doesn't work mskala @ 2004-05-18 22:26 ` mskala 2004-05-19 0:30 ` Eric McDonald 1 sibling, 0 replies; 7+ messages in thread From: mskala @ 2004-05-18 22:26 UTC (permalink / raw) To: xconq7 On Tue, 18 May 2004 mskala@ansuz.sooke.bc.ca wrote: > As far as I can tell, the (remove) form simply returns its second argument > instead of doing its job. Test case attached - this will fail regardless Further to that: it seems it works if the first argument (the thing being removed) is an atom. You can do 10-1=9, but not 10-3=7. Both my test case and apparently tailhook.g depend on the documented behaviour of removing a multi-element list from another multi-element list. -- Matthew Skala mskala@ansuz.sooke.bc.ca Embrace and defend. http://ansuz.sooke.bc.ca/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (remove) doesn't work 2004-05-18 22:12 (remove) doesn't work mskala 2004-05-18 22:26 ` mskala @ 2004-05-19 0:30 ` Eric McDonald 2004-05-19 12:24 ` mskala 1 sibling, 1 reply; 7+ messages in thread From: Eric McDonald @ 2004-05-19 0:30 UTC (permalink / raw) To: mskala; +Cc: xconq7 Hi Matthew, On Tue, 2004-05-18 at 16:12, mskala@ansuz.sooke.bc.ca wrote: > As far as I can tell, the (remove) form simply returns its second argument > instead of doing its job. Test case attached - this will fail regardless > because of the lack of terrain types, but it'll also give an error about > trying to match a length-7 list with another list that should also be > length-7 but is in fact length-10. The only shipped game module using > (remove) is tailhook.g. I haven't figured out whether (remove) is > misbehaving in that file, but it definitely misbehaves in my test case. I would definitely agree that 'remove' is broken, at least relative to its documentation. I too tried to use it once, but it was not giving the desired behavior, so I took a less convenient route that went around it. I didn't mention anything at the time, because I thought it was just some aspect of lispy behavior that I didn't fully comprehend (perhaps this business about only being able to remove atoms rather than lists). So, I guess the questions are, should it be able to remove lists? (It would certainly be nice, since that is the way most people, myself included, would want to use it) And, if so, how difficult would that functionality be to add into the GDL parser? Hopefully someone will have some insight into these questions, because otherwise I am going to have to do some more "context gathering". Regards, Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (remove) doesn't work 2004-05-19 0:30 ` Eric McDonald @ 2004-05-19 12:24 ` mskala 2004-05-19 15:44 ` Jim Kingdon 0 siblings, 1 reply; 7+ messages in thread From: mskala @ 2004-05-19 12:24 UTC (permalink / raw) To: Eric McDonald; +Cc: xconq7 On Tue, 18 May 2004, Eric McDonald wrote: > So, I guess the questions are, should it be able to remove lists? (It > would certainly be nice, since that is the way most people, myself > included, would want to use it) And, if so, how difficult would that > functionality be to add into the GDL parser? Hopefully someone will have > some insight into these questions, because otherwise I am going to have > to do some more "context gathering". It looks to me like it would be pretty easy to modify the eval() function near line 1432 of lisp.c, where it calls remove_from_list(), to detect when the thing being removed is a list, and in that case iterate over it. What concerns me is that this looks like the sort of thing that could have wacky side effects if modified, because even though the code looks straightforward right there, it seems to tie into LISP stuff that I don't understand. -- Matthew Skala mskala@ansuz.sooke.bc.ca Embrace and defend. http://ansuz.sooke.bc.ca/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (remove) doesn't work 2004-05-19 12:24 ` mskala @ 2004-05-19 15:44 ` Jim Kingdon 2004-05-19 21:10 ` mskala 0 siblings, 1 reply; 7+ messages in thread From: Jim Kingdon @ 2004-05-19 15:44 UTC (permalink / raw) To: mskala; +Cc: mcdonald, xconq7 > It looks to me like it would be pretty easy to modify the eval() function > near line 1432 of lisp.c, where it calls remove_from_list(), to detect > when the thing being removed is a list, and in that case iterate over it. Yeah. The only thing that spring to my mind is whether xconq has lists of lists, and whether you'd want remove (c d) from (a b (c d) e) to return (a b e) Off the top of my head I don't even remember whether xconq has lists of lists, much less whether the above functionality is important. > What concerns me is that this looks like the sort of thing that > could have wacky side effects if modified, because even though the > code looks straightforward right there, it seems to tie into LISP > stuff that I don't understand. I will point out that this kind of thing is quite easy to write unit tests for. Some of the existing stuff in autotest.c is strange, in the sense of not cleanly clearing out the game state before each test and getting things set up consistently. But that issue of game state isn't a problem for writing tests for lisp.c style code. Although writing tests won't completely prevent unintended consequences or bugs, it does make it easier to see what the code is supposed to do and verify that it continues to work as it evolves. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (remove) doesn't work 2004-05-19 15:44 ` Jim Kingdon @ 2004-05-19 21:10 ` mskala 2004-06-08 3:10 ` Eric McDonald 0 siblings, 1 reply; 7+ messages in thread From: mskala @ 2004-05-19 21:10 UTC (permalink / raw) To: Jim Kingdon; +Cc: xconq7 On Wed, 19 May 2004, Jim Kingdon wrote: > Yeah. The only thing that spring to my mind is whether xconq has > lists of lists, and whether you'd want > > remove (c d) from (a b (c d) e) to return (a b e) > > Off the top of my head I don't even remember whether xconq has lists > of lists, much less whether the above functionality is important. I'm sure xconq has lists of lists, but it seems to me that if you want to remove (c d) from (a b (c d) e) and return (a b e), you should use (remove ((c d)) (a b (c d) e)) not (remove (c d) (a b (c d) e)) . -- Matthew Skala mskala@ansuz.sooke.bc.ca Embrace and defend. http://ansuz.sooke.bc.ca/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: (remove) doesn't work 2004-05-19 21:10 ` mskala @ 2004-06-08 3:10 ` Eric McDonald 0 siblings, 0 replies; 7+ messages in thread From: Eric McDonald @ 2004-06-08 3:10 UTC (permalink / raw) To: mskala; +Cc: Jim Kingdon, xconq7 On Wed, 2004-05-19 at 15:08, mskala@ansuz.sooke.bc.ca wrote: > On Wed, 19 May 2004, Jim Kingdon wrote: > > Yeah. The only thing that spring to my mind is whether xconq has > > lists of lists, and whether you'd want > > > > remove (c d) from (a b (c d) e) to return (a b e) > > > > Off the top of my head I don't even remember whether xconq has lists > > of lists, much less whether the above functionality is important. > > I'm sure xconq has lists of lists, but it seems to me that if you want to > remove (c d) from (a b (c d) e) and return (a b e), you should use > > (remove ((c d)) (a b (c d) e)) > > not > > (remove (c d) (a b (c d) e)) . I just looked at a Common Lisp reference, and it sides with Jim's interpretation of the 'remove' keyword. Not that we have to follow Lisp or anything.... Perhaps we could implement a separate 'remove-list' keyword, which would follow your suggestion (and probably be more useful for GDL). If we assume that order of atoms in the two lists need not match, then the operation will effectively be O(n^2), which is probably not a big deal, since it would only be used during game module interpretation. Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2004-06-08 3:10 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-05-18 22:12 (remove) doesn't work mskala 2004-05-18 22:26 ` mskala 2004-05-19 0:30 ` Eric McDonald 2004-05-19 12:24 ` mskala 2004-05-19 15:44 ` Jim Kingdon 2004-05-19 21:10 ` mskala 2004-06-08 3:10 ` Eric McDonald
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).