Viewing Issue Simple Details Jump to Notes ] Wiki ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0008781 [DCSS] Patches tweak N/A 2014-07-11 22:40 2014-07-15 06:09
Reporter tswett View Status public  
Assigned To PleasingFungus
Priority normal Resolution done  
Status resolved   Product Branch 0.15 ancient branch
Summary 0008781: Make str_to_weapon use a map instead of a big long if/else chain
Description Currently, the str_to_weapon function in uses a big chain of ifs and elses to figure out what weapon a string refers to. This patch makes it look up the string in a map instead. The patch also strips whitespace from the string, so that users are free to (for example) refer to tomahawks as "toma hawks", or a hunting sling as a "huntingsling".
Additional Information
Tags No tags attached.
Attached Files ? file icon 0001-Make-str_to_weapon-use-existing-item-data.patch [^] (5,743 bytes) 2014-07-14 22:29 [Show Content]
? file icon 0001-Make-str_to_weapon-use-existing-item-data-2.patch [^] (6,042 bytes) 2014-07-14 22:57 [Show Content]

- Relationships

-  Notes
PleasingFungus (administrator)
2014-07-11 23:36

A few comments:

1. It'd be nice to have some doxygen comments at the start of both functions - check for style examples, such as they are.

2. We're still duplicating weapon names. It'd be really nice to do something like, perhaps, make a list of starting weapon types (I guess a hardcoded array), and iterate over them to populate most of the map, using (Presumably removing the spaces from those as well.) Then you'd only have to 'manually' populate the special names - "random", "variable", "thrown", etc...

3. What is the static_cast<int(*)(int)>(isspace) intended to accomplish?
tswett (reporter)
2014-07-12 01:17

I'll go about adding the Doxygen comments.

For the weapon names, do you think it would be fine to just iterate through all of the weapons? It wouldn't be optimal as far as computation time goes, but it would be simple. The special cases would be "staff" (short for "quarterstaff", not a magical staff), "sling" (short for "hunting sling"), and UNARMED, THROWN, RANDOM, and VIABLE.

As for the static_cast, I don't actually totally understand it myself. It wasn't working, and someone in ##c++ told me to add it, and then it worked. Apparently without the cast, the compiler can't resolve some overloading correctly. See [^] and [^]
PleasingFungus (administrator)
2014-07-12 01:32

Yeah, iterating through all the weapons would probably be fine; we filter out invalid weapons at a later step. It'd probably be simpler, actually. (Performance is unlikely to be a serious concern here.)
tswett (reporter)
2014-07-14 22:30

I just uploaded a new version of the patch. This one has Doxygen comments and uses the existing weapon data.
PleasingFungus (administrator)
2014-07-15 06:09

Looks good. Pushed, thanks!

- Issue History
Date Modified Username Field Change
2014-07-11 22:40 tswett New Issue
2014-07-11 22:40 tswett File Added: 0001-Make-str_to_weapon-use-a-map-instead-of-if-else.patch
2014-07-11 23:36 PleasingFungus Note Added: 0026733
2014-07-12 01:17 tswett Note Added: 0026734
2014-07-12 01:32 PleasingFungus Note Added: 0026735
2014-07-14 22:29 tswett File Added: 0001-Make-str_to_weapon-use-existing-item-data.patch
2014-07-14 22:30 tswett Note Added: 0026749
2014-07-14 22:35 PleasingFungus File Deleted: 0001-Make-str_to_weapon-use-a-map-instead-of-if-else.patch
2014-07-14 22:57 tswett File Added: 0001-Make-str_to_weapon-use-existing-item-data-2.patch
2014-07-15 06:09 PleasingFungus Note Added: 0026756
2014-07-15 06:09 PleasingFungus Status new => resolved
2014-07-15 06:09 PleasingFungus Fixed in Branch => 0.15 development branch
2014-07-15 06:09 PleasingFungus Resolution open => done
2014-07-15 06:09 PleasingFungus Assigned To => PleasingFungus

Mantis 1.1.8[^]
Copyright © 2000 - 2009 Mantis Group
Powered by Mantis Bugtracker