Solybum / PSOBBMod-Addons

GNU General Public License v3.0
29 stars 29 forks source link

Monster item mod (#1) #62

Open MachineHerald007 opened 1 year ago

MachineHerald007 commented 1 year ago
MachineHerald007 commented 1 year ago

example: https://vimeo.com/826568754?share=copy

Solybum commented 1 year ago

I have a few questions and suggestions regarding your PR.

  1. Splitting the fix for the double percent in a separate commit/PR because its unrelated to the drop chart feature.

  2. Make the drop charts separate from the addon, so that they could be used by other addons if necessary, and also make them in a way that they we can add other servers. You can see the item list for reference.

  3. Global variables, this is partly my fault since I left some like that, but either make them a local in the top of the file or pass it as parameters to the relevant functions.

  4. Seems like xSideMessage is the message in the right-ish of the screen you get when you enter a lobby? If so, this is not portable because this message might not be the same in all servers, could be disabled or just not have the content you are looking for, regardless, I'd recommend you change the way you obtain the lobby ID to retrieving the numeric value from memory, this way it will always be consistent no matter the server.

  5. I don't like that you are printing the rare drop content inside the getMonsterRareDrop function, I'd like if you can move the actual printing of the content to the code where that's being done, and just return the item name from the function.

  6. In the monster list you are not calling imgui.NextColumn(), I'd suggest you do so it keeps the columns structure, rather than printing the rare drop under the row.

  7. This last suggestions is a bit more complicated but, I'd prefer if you can handle the drop charts with ids instead of names, I do not like the text comparisons you are doing, this only works for English, if someone has the game in Spanish, Japanese, etc, it will not work, so using monster ids, item ids, retrieving item and monster names from the game memory will make this work regardless of the settings of the user or the server the user is on. While creating the drop charts with ids would be daunting by hand, we can use the servers ItemRT files to do so, I can provide Ultima's and I am sure Sodaboy would be ok with sharing that information with us.

Please contact me on discord Soly#0637, and we can discuss how to implement this stuff.

Hominghead commented 11 months ago

Any news on this implementation? Would like to have that

MachineHerald007 commented 9 months ago

Any news on this implementation? Would like to have that

Sorry, it's been a long time since I've worked/looked into this. Was busy with work/life, but you can still use this implementation since it should still work.

For a less technical/straight forward approach, just save a copy of the current addon that's based off Soly's main branch into a separate folder, then just download my branch as a zip, and replace your current addon folder with mine.

If there are big updates to main, just be sure to sync my fork with the main branch.

MachineHerald007 commented 9 months ago

I've been busy with work/life, so I've completely stopped working on these addons.