Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Vanilla

Reply
 
Thread Tools Display Modes
Old July 17, 2019, 03:16   #221
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,932
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by Nick View Post
At first glance this looks good. Anyone see any objections to it (if it works as described)?
It's cool that this is working, but I think a cleaner way to do this would be to set the 'key' member of struct object_menu_data differently when you're building the menu in build_obj_list(). textui_get_item() would have to pass along the 'command key' which can be found from the command via cmd_lookup_key(cmd, mode). This would have the advantage of not touching the menu code at all, and you keep all the logic in the one function. You could keep track of what letters are assigned when you build the list, and mark those that have no letter with '-', or just assign them using uppercase letters starting from A-Z I guess.
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Old July 17, 2019, 03:28   #222
Pete Mack
Prophet
 
Join Date: Apr 2007
Location: Seattle, WA
Posts: 5,348
Donated: $40
Pete Mack is on a distinguished road
@tak--
Agree. That stuff belongs under a function pointer, or outside the menu code entirely. It is really mysterious as a menu side effect.
Pete Mack is offline   Reply With Quote
Old July 17, 2019, 03:38   #223
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 54
Posts: 7,780
Donated: $60
Nick is on a distinguished road
Thanks all for the feedback, will look into following takkaria's advice.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old July 17, 2019, 04:44   #224
jml34
Rookie
 
Join Date: Jul 2019
Posts: 11
jml34 is on a distinguished road
Quote:
Originally Posted by Sideways View Post
until code was written to make sure every item always has a letter.
What does that code do? For each hidden item, assign the first free letter?
My quick patch doesn't update the menu keys, which is indeed pretty confusing.
I'll look into that as soon as I'm done with trying to get stacking limits to work
EDIT Ah, so many posts while I was thinking. Er, do I still need to look into it or are knowledgeable people going to do it? Never mind , looks like the big boss is on it...

Last edited by jml34; July 17, 2019 at 04:51.
jml34 is offline   Reply With Quote
Old July 17, 2019, 08:33   #225
jml34
Rookie
 
Join Date: Jul 2019
Posts: 11
jml34 is on a distinguished road
I guess it's OK to double-post for unrelated matters?
I'm playing around with inscriptions for stacking limits and auto_pickup. So {!L25} would only stack up to 25 when walking over a stackable stack of objects.
The get command happily ignores this so far. I think it should prompt for a quantity when picking the most it can would overshoot the limit.

It's not thoroughly tested at all but I thought I'd ask for an opinion. I don't exactly get how the code is organized (lots of similar code in several places) so I'm afraid I'm adding to the mess. It's just a bit annoying when the game keeps stacking things, espacially when it uses a new slot, but no major problem there.

I think it's sane to ignore !Lxx when xx>40. Do people really want to have 60 or 80 of something? It would take more coding to check this out, since as it is, only the first stackable slot is checked out, so if it says !L42, then it will pickup until "it's at 42" which means 2 will go to the next stack each time. I guess I could just check what happens with the next inventory slot...

There's potential for confusion if you manually overshoot the limit, then drop some of them, then change your mind about the limit, then manually pickup items with the previous inscription etc., you end up with several stacks with different inscriptions. Only the first stack is taken into account so if you have 8 items {!L08} then 2 items {!L05}, you won't pickup anything when walking over uninscribed items. Not sure what to make of that. What if there was 7 items !L08, would I have to add 1 item to this stack then again several items to the next? Huh...

---
So the patch I'm trying is in cmd-pickup.c
There's something weird with player_pickup_aux. There are 3 calls to this function. Parameter auto_max is unused 2 times (=0), and is set to inven_carry_num(obj,true) in the 3d, in the do_autopickup function. Now in obj-gear.c we can see that, for the inven_carry_num function, stack=true only serves to return 0 when obj can't be stacked to an existing inventory item.
So either auto_max=0, or the first thing player_pickup_aux does, calling inven_carry_num(obj,false), is to recompute its value.
So auto_max should perhaps be a bool then. But I don't really get in what situation we get to that part of the code with the call to get_quantity so meh.
I need to set a pickup limit somewhere though, so I just added a new parameter :
Code:
static void player_pickup_aux(struct player *p, struct object *obj,
                                          int auto_max, int pickup_limit, bool domsg)
{
        int max = inven_carry_num(obj, false);

        /* Confirm at least some of the object can be picked up */
        if (max == 0)
                quit_fmt("Failed pickup of %s", obj->kind->name);

        /* Set ignore status */
        p->upkeep->notice |= PN_IGNORE;

/* new code */
        if(pickup_limit && max>pickup_limit) max = pickup_limit;
/* end of new code */

        /* Carry the object, prompting for number if necessary */
        if (max == obj->number) {
                if (obj->known) {
                        square_excise_object(p->cave, p->py, p->px, obj->known);
                        delist_object(p->cave, obj->known);
                }
                square_excise_object(cave, p->py, p->px, obj);
                delist_object(cave, obj);
                inven_carry(p, obj, true, domsg);
        } else {
                int num;
                bool dummy;
                struct object *picked_up;

/* old code
                if (auto_max)
                        num = auto_max;
                else
                        num = get_quantity(NULL, max); */
/* new code */
                if (!auto_max) num = get_quantity(NULL, max);
                else num = max;
/* end of new code */
                if (!num) return;
                picked_up = floor_object_for_use(obj, num, false, &dummy);
                inven_carry(p, picked_up, true, domsg);
        }
}
[...]
        /* We're given an object - pick it up */
        if (obj) {
                player_pickup_aux(p, obj, 0, 0, domsg); // new code: add a 0 param
[...]
        /* Pick up object, if legal */
        if (current) {
                /* Pick up the object */
                player_pickup_aux(p, current, 0, 0, domsg); // new code: add a 0 parameter
[...]
int do_autopickup(struct player *p)
{
[...]
                        if (auto_pickup_okay(obj)) {
                                /* Pick up the object (as much as possible) with message */
/* Old code 
                                player_pickup_aux(p, obj, inven_carry_num(obj, true), true);
                                objs_picked_up++; End of old code */
/* New code */
                                int num_to_pickup = inven_carry_num(obj, true);
                                /* Check first if there's a stacking limit however */
                                const struct object *gear_obj = find_stack_object_in_inventory(obj);
                                int stack_limit = -1;
                                if (check_for_inscrip(gear_obj, "!L")) {
                                    const char *s = quark_str(gear_obj->note); // gear_obj->note is not 0
                                    while (s) {
                                        s = strstr(s, "!L");
                                        /* The stacking limit tag is !L<digit><digit> */
                                        if (s && '0'<=s[2] && s[2]<='9' && '0'<=s[3] && s[3]<='9') {
                                            stack_limit = (int)10*(s[2]-'0') + s[3] - '0';
                                            /* TODO ignore staking limits > 40? */
                                            break;
                                        }
                                        if (s) s++;
                                    }
                                    if (stack_limit>=0)
                                            num_to_pickup = MIN(num_to_pickup, stack_limit - gear_obj->number);
                                }
                                if (num_to_pickup>0) {
                                    player_pickup_aux(p, obj, num_to_pickup, num_to_pickup, true);
                                    objs_picked_up++;
                                }
/* End of new code */
                        }
(hum, no spoiler tags on this forum? I wanted to hide the code)
On a side note, I probably won't have much time until next month

Last edited by jml34; July 17, 2019 at 08:53.
jml34 is offline   Reply With Quote
Old July 17, 2019, 13:20   #226
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,932
Donated: $40
takkaria is on a distinguished road
Code:
if (check_for_inscrip(gear_obj, "!L")) {
    const char *s = quark_str(gear_obj->note); // gear_obj->note is not 0
    while (s) {
        s = strstr(s, "!L");
        /* The stacking limit tag is !L<digit><digit> */
        if (s && '0'<=s[2] && s[2]<='9' && '0'<=s[3] && s[3]<='9') {
            stack_limit = (int)10*(s[2]-'0') + s[3] - '0';
            /* TODO ignore staking limits > 40? */
            break;
        }
        if (s) s++;
    }
    if (stack_limit>=0)
            num_to_pickup = MIN(num_to_pickup, stack_limit - gear_obj->number);
}
This is dangerous code with a potential out of bounds access. I think the following will do what you want just as well:

Code:
const char *inscription = quark_str(gear_obj->note);
const char *s = strstr(inscription, "!L");
if (s && sscanf(s, "!L%d", &stack_limit) > 0) {
    num_to_pickup = MIN(num_to_pickup, stack_limit - gear_obj->number);
}
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Old July 17, 2019, 15:06   #227
Sideways
Swordsman
 
Join Date: Nov 2008
Posts: 421
Sideways is on a distinguished road
Quote:
Originally Posted by jml34 View Post
What does that code do? For each hidden item, assign the first free letter?
I think it does that, yes; though there's unrelated but very similar code elsewhere that essentially works by swapping letters between the competing items. (That approach sometimes leads to unintuitive results if an item's letter gets swapped multiple times during the process.)

The fundamental difference between the Frog/Pos code and V is that in Frog every item only has one corresponding key in a given menu (the inscriptions only change what that key is), while V has (so far) tried to make both the original letter and the inscribed key apply within the same menu. I think the latter approach is ultimately unworkable with @[letter][letter] inscriptions allowed, though it didn't cause any conflicts as long as the inscribed keys were always numbers.

(I'd also say removing that is a very cheap price to pay for making @[letter][letter] inscriptions work But that might be because I'm already used to it.)
__________________
The Complainer worries about the lack of activity here these days.

Last edited by Sideways; July 17, 2019 at 15:14.
Sideways is offline   Reply With Quote
Old July 17, 2019, 16:39   #228
jml34
Rookie
 
Join Date: Jul 2019
Posts: 11
jml34 is on a distinguished road
@takkaria If you mean the accesses to s[2] and s[3] they can't be out of bounds (null-terminated string so these expressions are in bound when evaluated) -- I copied this from elsewhere in the code.
Now your code is much cleaner, and faster too. Er, I think you could have a look at some elsewheres in the code
I just quickly tested it with
Code:
if (s && sscanf(s, "!L%d", &stack_limit) > 0 && 0<stack_limit &&
                      stack_limit<=gear_obj->kind->base->max_stack) {
and it worked (ignoring !L with numbers>40).
I haven't coded in C for like 20 years so I also tested !L0x20 but it seems %d doesn't accept hexa format

Another quick note (I realize I wasn't clear): I think the player_pickup_aux code would work the same without the 4th parameter and
Code:
                if (!pickup_limit) num = get_quantity(NULL, max);
                else num = max;
(don't use auto_max since it's unused). Silly me just got scared of that call to get_quantity that I didn't understand so I left auto_max for no good reason that I know of...

------------
About @<cmd><letter>, my plan would be to have the menu scan the keys, check if they're hidden by an entry in the inscriptions array, in which case the inscriptions array would be scanned for an entry that points to the item: this key would be displayed in the menu if there's one, if not, the first free slot would be assigned first. Degenerate case: the player engraved multiple @ commands to objects so there's no free slot available, we display a space in the menu and the object is not reachable.
@Sideways So @<cmd><letter> could actually be made to work with multiple ways to select an item. Use a key displayed in the menu -> select the corresponding item, use some other key -> select the first engraved item if any.
Now I don't know if that's a desirable feature, the code could be messy, perhaps I can't get back to it before the 1st of 08, and above all other people have said there's a more desirable way to have @<cmd><letter> work so I'll leave it at that for now

Last edited by jml34; July 17, 2019 at 16:47.
jml34 is offline   Reply With Quote
Old July 17, 2019, 17:26   #229
khearn
Rookie
 
Join Date: Jul 2007
Posts: 18
khearn is on a distinguished road
Yeah, I think not being able to use an item because something else has its letter in an engraving does seem like it would be a problem at times. There needs to be some way to tell the system to 'ignore engravings for this command'.

Maybe <cmd>'<letter>? So "qq" would quaff the potion engraved with @qq, and "q'q" would quaff the potion in inventory slot q. And of course, if there is no @qq engraving, "qq" would just quaff the potion in slot q.

Last edited by khearn; July 17, 2019 at 17:37.
khearn is offline   Reply With Quote
Old July 17, 2019, 18:00   #230
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,932
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by khearn View Post
Yeah, I think not being able to use an item because something else has its letter in an engraving does seem like it would be a problem at times. There needs to be some way to tell the system to 'ignore engravings for this command'.

Maybe <cmd>'<letter>? So "qq" would quaff the potion engraved with @qq, and "q'q" would quaff the potion in inventory slot q. And of course, if there is no @qq engraving, "qq" would just quaff the potion in slot q.
I think this is over-complicated. If the user has assigned a different key to an item for a particular command, the item that has been overriden should just be assigned another key that isn't in use - I liked the suggestion of uppercase letters.
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
First release Quirk Sil 18 November 14, 2017 22:45
3.4.0 Release Candidate is available fizzix Vanilla 111 July 5, 2012 12:39
3.2 release candidate is upon us! d_m Vanilla 147 January 19, 2011 10:10
Release HellBand 0.8.7 konijn_ Variants 4 December 27, 2009 04:14
reactions to changes in new V release will_asher Vanilla 50 April 9, 2008 18:47


All times are GMT +1. The time now is 12:40.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.