Bugzilla – Bug 3096
menu_addblank slot=1 does not count in prop per page
Last modified: 2014-02-08 23:52:23 PST
I don't know if I can explain this well or even if this is really an issue. When using menu_addblank with default slot 1, it uses up a number in the list but does not count as a prop per page. Whereas menu_addblank with slot 0 seems to be fine not counting as a prop because the list gets done right. ie. menu_addblank with slot 1 and default prop per page at 7, with a blank using number 4: <code>1. blah 2. blah 3. blah 5. blah 6. blah 7. blah 8. blah 9. Back 0. Next 11. Exit</code> Thing is I'm not completely sure how this would effect use of a blank as a prop per page in other situations. I haven't really done any testing with this.
Having looked into the issue I think this is a design flaw that can't be fixed easily with the old functions. Blanks (and text, which also exhibits this bug) are affecting the ordering of actual items when slot is set to 1. The issue is that slot eating blanks are a special type of blanks when probably they should be a special type of item - i.e. an item that has no text. This is also shown by the fact that addblank and addtext are checking for GetItemCount() >= 10 if slot is 1 when they don't even contribute to m_Items growing. You can add an indefinite amount of them, screwing up the menus. To fix this I propose to have a special type of item to achieve this. Changing blanks to be items all the time or only when slot is 1 would change what old plugin code has to expect about menu_items or menu_pages, so it is likely that this change would break compatibility somewhere. The way I see it we have two options: 1) Add a new parameter 'blank' to menu_additem; I think this is inconvenient because menu_additem already has three optional parameters that would not even matter to blanks because they can't be selected anyway 2) Add new functions like menu_add[blank|text]2 or menu_addtextitem to add this type of item This would add the desired functionality for new plugins - that then have to deal with the fact that slot eating blanks and texts are real items and affect the menus item count and pagination - while leaving old plugins alone. I hope I'm not missing stuff here :)
Created attachment 3407 [details] [review] menu_addblank2, menu_addtext2 Example patch if adding menu_addblank2/addtext2 is an acceptable solution to this problem.
Created attachment 3408 [details] Test plugin with verifier for patch Commands singleblank/manyblanks display faulty behavior for two different kinds of blank/text usage. Commands singleblank_fix/manyblanks_fix display how the proposed patch fixes the issues.
Comment on attachment 3407 [details] [review] menu_addblank2, menu_addtext2 Review of attachment 3407 [details] [review]: ----------------------------------------------------------------- Looks good for me. After checking the issue, I would agree with Valentin. Fixing it without new natives would be tricky, adding unnecessary complexity in the code and would lead most likely to break things. Simple & safer to rely on new natives dealing properly with blanks as empty string. Patch tested and is working as espected. Asking feedback? from joropito.
Splinter is having trouble reading these patches. Could you add this to your hgrc? [defaults] diff = -U 8 -p qdiff = -U 8 -b [alias] qstatus = status --rev -2:. [diff] git = 1 showfunc = true unified = 8
Err, ignore the [alias] section.
Created attachment 3410 [details] [review] menu_addblank2, menu_addtext2 (new format) Does this work?
It seems not. :P Let me try.
Created attachment 3415 [details] [review] Proposed patch (splinter view ready) Patch looks good. I've also tested and is working as expected.
Created attachment 3416 [details] [review] Proposed fixed patch Actually it misses the include modification.
Created attachment 3418 [details] [review] Proposed fixed patch Better documentation (by Nextra).
Pushed changeset: http://hg.alliedmods.net/amxmodx-central/rev/51bf0d8e210b Changelog: Add menu_addblank2 and menu_addtext2 to fix unexpected behavior with the original ones when slot=1 (bug 3096, r=Arkshine)
Commit pushed to master at https://github.com/alliedmodders/amxmodx https://github.com/alliedmodders/amxmodx/commit/21a00e00a27ee31311f6d51e62388c94069bd930 Add menu_addblank2 and menu_addtext2 to fix unexpected behavior with the original ones when slot=1 (bug 3096, r=Arkshine) Former-commit-id: b4f84a5cee58d4a8fad716af82bca5b8aa0f5ff2