|Anonymous | Login | Signup for a new account||2019-02-20 23:29 CET|
|Main | My View | View Issues | Change Log | Wiki | Tavern | News|
|Viewing Issue Simple Details|
|ID||Category||Severity||Reproducibility||Date Submitted||Last Update|
|0008781||[DCSS] Patches||tweak||N/A||2014-07-11 22:40||2014-07-15 06:09|
|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 initfile.cc 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".|
|Tags||No tags attached.|
0001-Make-str_to_weapon-use-existing-item-data.patch [^] (5,743 bytes) 2014-07-14 22:29 [Show Content]
0001-Make-str_to_weapon-use-existing-item-data-2.patch [^] (6,042 bytes) 2014-07-14 22:57 [Show Content]
A few comments:
1. It'd be nice to have some doxygen comments at the start of both functions - check godblessing.cc 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 itemprop.cc:item_base_name(). (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?
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 http://ideone.com/0Miw8Y [^] and http://ideone.com/8A5LMQ. [^]
|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.)|
|I just uploaded a new version of the patch. This one has Doxygen comments and uses the existing weapon data.|
|Looks good. Pushed, thanks!|
|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|