Add a PluginIterator methodmap

[Deathreus]

This follows the behavior of OOP

Full disclosure, I wrote this a month ago and forgot
BONUS: Fix a stray space...

[JoinedSenses]

methodmap is mixing tabs and spaces at beginning.

methodmaps through SM are using double slash comments as well rather than /**

I think it would make sense to mirror the behavior of the CommandIterator/FrameIterator with a method called Next and a property or method to retrieve the current value.

[Deathreus]

Haha, copy-paste go brr

I concur that a Plugin property makes more sense than following the native naming

[Headline]

LGTM, will let this cool before I merge

[peace-maker]

Meh, this is weird, since this iterator behaves differently from other iterator methodmaps like CommandIterator and FrameIterator.

This one advances the iterator when reading a property and only returns if there are more in the Next() function. The other ones advance the iterator in the Next() function and allow you to read the values of the current iteration multiple times. I think the latter behavior is more intuitive and we should reimplement the plugin iterator properly while we have the chance here!

See #819

[Deathreus]

I thought that MorePlugins advanced the iterator, huh

I should have something better ready by tonight

[Deathreus]

I think this looks sane

[peace-maker]

Thank you! This looks better, but the usage pattern of a while(it.Next()) loop would skip the first plugin at the moment.

PluginIterator it = new PluginIterator();
while (it.Next()) {
    char pluginName[128];
    GetPluginFilename(it.Plugin, pluginName, size of(pluginName));
    PrintToServer("%s", pluginName);
}
delete it;

[Deathreus]

Interesting, I'll have to think on that as I don't think changing core interface behavior would be advised

I was in the mindset of for loops as that's usually what I do iterators in

[psychonic]

The change would be able to be made for only the methodmap version, to be consistent with the other methodmap iterators, while keeping the non-methodmap behavior as-is so to not break existing plugins.

[Deathreus]

Sanity check

[peace-maker]

The non-methodmap API is now incompatible with the methodmap api. An iterator created with GetPluginIterator still skips the first entry if you use the methodmap api afterwards.

Maybe it's better to change the non-methodmap implementation to use the new iterator class as well to be consistent. Otherwise we'd need to create a new handle type.

[Deathreus]

Ah yeah, I missed this

[peace-maker]

Thank you, this looks solid now!

GetPluginIterator returns an handle of the same handletype as PluginIterator(), so it could potentially be used with the methodmap api after view_as<PluginIterator>, but that is such an obviously unnecessary pattern that I'm fine to blame the user in this case :D

We can switch GetPluginIterator to use the new iterator and return a PluginIterator in a follow up patch if needed.

[Deathreus]

👍