Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Add RoF2 Guild features #3699

Merged
merged 78 commits into from
Feb 10, 2024
Merged

Conversation

neckkola
Copy link
Contributor

@neckkola neckkola commented Nov 19, 2023

Good afternoon.

Over the past several months I have been working on adding the RoF2 Guild Features. While it still requires some spit and polish, I believe it is ready for review. I need to clean it up, rewrite some functions, so please be gentle. :-)

I did not re-write everything as I tried to use the existing guild code as-is. That said, happy to update as you see fit, particularly using the new patterns and repositories. I learned a lot from my last PR that needs to be implemented here. I don't want to wait any longer though, so thought I would push it today.

This adds the following guild features and design pattern the existing guild system;

  • guild features are based on RoF2 within source with translators used to converted between client differences
  • backward compatible with Ti and UF, and allows for mixed client servers
  • Guild Bank for Ti and UF is based on RoF2 Permissions for banking so that the Guild Leader does not have to use RoF2 Client
  • Guild Ranks and Permissions are enabled.
  • Guild Tributes are enabled.
  • Event logging via rules for donating tribute items and plat
  • Rules to limit Guild Tributes based on max level of server
  • Rewrote guild communications to client using specific opcodes
    -- Server no longer sends a guild member list on each zone
    -- Guild window is updated when a member levels, rank changes, zone changes, banker/alt status using individual opcodes
    -- When a member is removed or added to a guild, a single opcode is sent to each guild member
    -- This reduces network traffic considerably

Known issues:

  • Visual bug only. Guild Tributes window will display a 0 for level if tribute is above max level rule setting.
  • Visual bug only. Guild Mgmt Window will not display an online member within the Notes/Tribute tab if the player has 'show offline' unchecked and a guild member zones This is resolved by selecting and de-selecting the 'Show Offline' checkbox.

There are database changes that rewrite the guild_members structure and DATA, so please only use in a testing environment. There is also a sql required to update the guild tribute items. This is not within source and is posted here. It creates several items using id 150000 and above so if you have items in that range, use caution. Suggest a test db as well, or a backup before hand.
GuildTributes_2023_09_13_Complete.zip

You will also notice a few #guild test commands. These were/are for my testing and will be removed once the PR is finalized.
There are also a few updates to the world api for testing purposes. These too will be removed.
And finally, the lua protocol opcode item is also fixed, again for my testing purposes.

This code has been working in a few environments for several months, therefore the functionality has been tested. That said, please post any issues and I will look to resolve them. Appreciate any testing that you can do.

For the devs, thanks in advance for taking a look at this and I hope it helps the community.

Neckkola.

@fryguy503
Copy link
Contributor

FWIW - I have been running this code for months? now and its current state is very stable and the functionality is much appreciated by players.

Thank you so much Neckkola!

@noudess
Copy link
Contributor

noudess commented Nov 26, 2023

Any chance you know how to fix mail to guild? It dies after something like 50 members.

@neckkola
Copy link
Contributor Author

@noudess I will certainly take a look a see.

@neckkola
Copy link
Contributor Author

@noudess. Take a look at #3724. Be appreciative if you can test and let me know.

common/item_instance.h Outdated Show resolved Hide resolved
common/patches/uf.cpp Outdated Show resolved Hide resolved
common/patches/uf.cpp Outdated Show resolved Hide resolved
common/patches/uf.cpp Outdated Show resolved Hide resolved
common/patches/uf.cpp Outdated Show resolved Hide resolved
common/patches/uf.cpp Outdated Show resolved Hide resolved
common/patches/uf.cpp Outdated Show resolved Hide resolved
common/guild_base.cpp Outdated Show resolved Hide resolved
zone/client_packet.cpp Outdated Show resolved Hide resolved
zone/guild_mgr.cpp Outdated Show resolved Hide resolved
zone/guild_mgr.cpp Outdated Show resolved Hide resolved
zone/guild_mgr.cpp Outdated Show resolved Hide resolved
zone/mob.cpp Outdated Show resolved Hide resolved
zone/mob.cpp Outdated Show resolved Hide resolved
@Kinglykrab
Copy link
Contributor

Left some comments, client_packet.cpp seems to have had the entire file accidentally refactored.

@noudess
Copy link
Contributor

noudess commented Nov 29, 2023

@noudess. Take a look at #3724. Be appreciative if you can test and let me know.

Will do.

@noudess
Copy link
Contributor

noudess commented Nov 29, 2023

@noudess. Take a look at #3724. Be appreciative if you can test and let me know.

Tested and works fine here!!

Fixed: #577 (open since 2016)!!!

Thanks!!

Copy link
Contributor

@hgtw hgtw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For comments on using zone spawn packets instead of the appearance updates on client connection: if it's too difficult to change these now it could be done in a followup if you don't want to tackle it but it would probably significantly reduce packets on client connections for guild updates. From only looking at it I'm not sure if it would be an easy switch or not.

zone/entity.cpp Outdated Show resolved Hide resolved
zone/entity.cpp Outdated Show resolved Hide resolved
zone/client_packet.cpp Outdated Show resolved Hide resolved
zone/client_packet.cpp Outdated Show resolved Hide resolved
zone/entity.cpp Outdated Show resolved Hide resolved
common/eq_packet_structs.h Outdated Show resolved Hide resolved
zone/guild.cpp Outdated Show resolved Hide resolved
Add show guild tag to default spawn process
Removed several unused functions as a result
Updated MemberRankUpdate to properly update guild_show on rank change.
Updated OP_GuildURLAndChannel opcode for UF/RoF2
@neckkola
Copy link
Contributor Author

@hg Thanks for the comments regarding zone spawns. I had a look and have utilized that functionality to remove the specific sendappearance packets. This has allowed for several functions to be removed. Please let me know if you see anything else. Once ok, I will rerun my tests.

@hgtw
Copy link
Contributor

hgtw commented Jan 26, 2024

Nothing else jumps out with the zone packet stuff so I think you're good to run tests there after fixing the ScanCloseMobs/SendGuildSpawnAppearance line removal swap issue.

The only other major issues left I think are the world comments (mainly the possible GetCLE() nullptr pointer issue and consolidating all the duplicate booted zone server packet sends). There's also a few more strn0cpy changes for overflow safety that wouldn't hurt.

Outside of those the only concern would be some of the db queries that occur in loops instead of being batched for outside, but I'm not sure if those are low frequency updates or not.

Created function for repetitive zonelist sendpackets to only booted zones
Re-Inserted accidental delete of scanclosemobs
@neckkola
Copy link
Contributor Author

Good morning @hgtw

I have reviewed the world comments and believe I have them updated to resolve the comments and re-inserted the drop scanmobs item. Apologize for that oversight.

Also repaired a few other items that I saw, and updated all the remaining strcpy items and left some notes for the db loop items.

@Kinglykrab I believe I have the formatting items resolved as well.

Anything further, please let me know as I may have overlooked something.

Copy link
Contributor

@hgtw hgtw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any major issues so as long as it tests ok after the changes it looks good. Nice work.

@neckkola
Copy link
Contributor Author

Awesome. I will do some testing on my end. Would welcome others doing the same given the size and scope of the changes.

Removed a duplicate db call
Fixed a fallthrough issue
@neckkola
Copy link
Contributor Author

I ran through several test cases between RoF2, UF and TI. Found a few items along the way.
Also loaded JJ's db updates (thank you!) and all seems good there. I have not tested all the tributes, though @fryguy503 has been running them for some time with no reports of issue.

Copy link
Contributor

@Kinglykrab Kinglykrab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, think it's good to go. Will let @Akkadius take another once over if he wants.

@neckkola
Copy link
Contributor Author

Do the db changes need to be pushed to peq_content?

@noudess
Copy link
Contributor

noudess commented Jan 27, 2024

Do the db changes need to be pushed to peq_content?

Does this mean people need db changes if we dont use currect pew content?

zone/guild_mgr.cpp Outdated Show resolved Hide resolved
@Akkadius
Copy link
Member

Akkadius commented Feb 7, 2024

GuildTributes_2023_09_13_Complete_Updated.zip

Updated item ids to match main database. No need to add the new items as we have a pull for it from 13th Floor already.

Added these on PEQ

@Akkadius
Copy link
Member

Akkadius commented Feb 7, 2024

Do the db changes need to be pushed to peq_content?

This is done

@Akkadius Akkadius merged commit 91f5932 into EQEmu:master Feb 10, 2024
1 check passed
@Akkadius Akkadius mentioned this pull request Feb 10, 2024
joligario added a commit to ProjectEQ/peqphpeditor that referenced this pull request Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants