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

Adhoc Multiplayer Chat Support #7451 #9090

Merged
merged 23 commits into from
Mar 8, 2020
Merged

Conversation

adenovan
Copy link
Contributor

Android and Windows basic chat function is ready , no need for rush to merge, there is some WIP improvement the details is on #7451 , i am sorry if my code style is bad im new to C++ and google code style.

@adenovan
Copy link
Contributor Author

@sum2012 do you know where is the code to add hotkey on windows ? i want add CTRL + C to open the chatscreen

@hrydgard
Copy link
Owner

If you add a Windows menu item, you can add a hotkey in the "accelerator table" resource. Look in the Win32 resources.

@sum2012
Copy link
Collaborator

sum2012 commented Oct 24, 2016

@adenovan can you also add windows OSK ? (When "checked enable windows native keyboard and not in full screen : if (g_Config.bBypassOSKWithKeyboard && !g_Config.bFullScreen))
see https://github.com/hrydgard/ppsspp/blob/master/Core/Dialog/PSPOskDialog.cpp#L807

@adenovan
Copy link
Contributor Author

@sum2012 did you mean this windows OSK ? its already working sum
screenshot 125

@hrydgard thanks for the info the hotkey is added because of that :D , but i can't set the focus to the Text Edit automaticly when showing the chat screen , tried to change focus on this line but no luck yet https://github.com/adenovan/ppsspp/blob/b19c408a8e72d060f58ba7d5dcd57b8e7d27eb57/UI/ChatScreen.cpp#L57

@sum2012
Copy link
Collaborator

sum2012 commented Oct 25, 2016

@adenovan I mean this
1

@adenovan
Copy link
Contributor Author

@sum2012 is that OSK really needed ? if not full screen i think current implementation is better than poping up another text box from Text Edit UI again. What version of windows use that OSK? did windows 7 can't chat when not fullscreen?

@sum2012
Copy link
Collaborator

sum2012 commented Oct 25, 2016

@adenovan Yes,I think we need it for easier copy and plaste ,input chinese word,etc.
It is by option "Enable Windows native keyboard in "System" in PPSSPP,

@adenovan adenovan force-pushed the rechat branch 4 times, most recently from d942696 to 60c25f3 Compare October 26, 2016 23:15
@adenovan
Copy link
Contributor Author

adenovan commented Oct 26, 2016

thanks for your fix on #9095 @hrydgard , this feature is ready i think, can you review my code if you have spare time?

@sum2012 i can't figure it out how that OSK works and use it, tried enable native keyboard and not in fullscreen then change nickname nothing happened :D , how to use that sum can you teach me before i dive into the code.

@sum2012
Copy link
Collaborator

sum2012 commented Oct 27, 2016

@adenovan done ,adenovan#2
OSK pop up once,But I want to make it simple.

@sum2012
Copy link
Collaborator

sum2012 commented Oct 27, 2016

@adenovan Build fix for Linux QT adenovan#3

Core/Config.h Outdated
@@ -394,6 +394,8 @@ struct Config {
bool bEnableAdhocServer;
int iWlanAdhocChannel;
bool bWlanPowerSave;
bool bEnableNetworkChat;
int iNewChat;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is iNewChat and why is it part of the ppsspp config struct? It looks like it may be for counting how many new chat messages there are, but then it probably doesn't belong here.

-[Unknown]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iNewChat is a counter used when chatscreen is not visible to notify user there is a chat coming from other user. if the chatscreen is visible it will not increment it and will call the pointer to update the chatsreen. the counter is displayed on the choice with value display https://github.com/hrydgard/ppsspp/pull/9090/files#diff-4497b3f963a2b3d65020d48fb909f4adR771 . im just copying another choice with value display line look at the code, and found its used config.h so i use that file for global too. ah i didn't notice that line inside and part of ppsspp config struct. should i move it to proadhoc.h and extren that variable so i can used that new chat variable on emuscreen and chatscreen? move it to proAdhoc.h it is the right choice ?

@@ -983,6 +988,54 @@ void freeFriendsRecursive(SceNetAdhocctlPeerInfo * node) {
free(node);
}

//@params chatmenu pass NULL on destroy, and pass ChatMenu On Create (EmuScreen.cpp)
void setChatPointer(ChatMenu * chatmenu) {
if (chatmenu != NULL) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete does nothing if its argument is null. So you don't need this check.

-[Unknown]

Copy link
Contributor Author

@adenovan adenovan Oct 31, 2016

Choose a reason for hiding this comment

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

this pointer really give me a headache its only quick workaround to update the chat screen ui from proadhoc.cpp when new chat send or new chat is coming that i found. didn't know how to properly linking class and file on C++ because im still a newcomer in C++ programming and PPSSPP development. this pointer also cause a random crash with access violation reading location or access violation writing location. so im using null as workaround on chatscreen destructor. but i think it cause memory leak and try that if argument delete but it still crashed on low end device (already publish a build to online community for helping me testing the code) most of them report 1 or 2 random crash occured in 1 day. i will try remove this function completely and implement your below comment it looks a good solution thanks for reviewing :)

@@ -32,6 +32,7 @@
#include "Core/HLE/sceKernel.h"
#include "Core/HLE/sceKernelMutex.h"
#include "Core/HLE/sceUtility.h"
#include "UI/ChatScreen.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't include UI from inside Core. The best option is to add checking in the ChatScreen class, inside its update() method, which is called each frame. Since the count won't be drawn until then anyway, no need to update more frequently.

It could call a GetChatCount() function, or something, and then you wouldn't need to abuse the g_Config object for that value (which isn't really configuration.) Also, setChatPointer would probably be gone, making managing the lifetime of that object completely UI's job - which is simpler.

-[Unknown]

Copy link
Contributor Author

@adenovan adenovan Oct 31, 2016

Choose a reason for hiding this comment

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

ok i will try to not include the ui and make the chats update work from chatscreen class. Edit thanks its works. i think we didn't need GetChatCount() function or similiar its already worked with just a boolean to updating the chat screen from proAdhoc.cpp.

@@ -68,6 +68,7 @@ BEGIN
"W", ID_EMULATION_STOP, VIRTKEY, CONTROL, NOINVERT
"B", ID_EMULATION_RESET, VIRTKEY, CONTROL, NOINVERT
"T", ID_EMULATION_CHEATS, VIRTKEY, CONTROL, NOINVERT
"C", ID_EMULATION_CHAT, VIRTKEY, CONTROL, NOINVERT
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use spaces in this file. You might install the editorconfig plugin for your editor of choice.

Also below.

-[Unknown]

Copy link
Contributor Author

@adenovan adenovan Oct 31, 2016

Choose a reason for hiding this comment

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

i add this hotkey by using visual studio view code. how to disable that editorconfig or i must edit this code from another editor like notepad++ ? or add it from resource view on visual studio?

UI/EmuScreen.cpp Outdated
@@ -356,6 +357,11 @@ void EmuScreen::sendMessage(const char *message, const char *value) {
} else {
gstate_c.skipDrawReason &= ~SKIPDRAW_WINDOW_MINIMIZED;
}
} else if (!strcmp(message, "chat screen")) {
releaseButtons();
ChatMenu * ch = new ChatMenu();
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if I press the button while there's already a chat menu? Does it make more sense to just keep the existing chat menu?

-[Unknown]

Copy link
Contributor Author

@adenovan adenovan Oct 31, 2016

Choose a reason for hiding this comment

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

by keep the existing chat menu did you mean,
ChatMenu * ch = new ChatMenu() ; <-- create this object one times on emu screen
screenManager()->push(ch); <-- and call this line on ctrl + c or press the chat button

or you mean create the class like this
screenManager()->push(new ChatMenu());

because we will not use setPointer() again, so its better to not create chat menu pointer right?

Nothing happened when thereis already a chat menu visible and press ctrl + c again. it didn't crash or antoher chat menu appeared but im not sure the chat menu object is created again or use the previously created chat menu.

@hrydgard
Copy link
Owner

This needs a rebase before merging (there is some small conflict in proAdhoc.cpp).

Other than that, code isn't perfect but if it works I'm alright with it :)

@adenovan
Copy link
Contributor Author

@hrydgard its working for basic chat function and found no random crash on 1 month testing. i force my server community to use my PPSSPP build they seems happy with the chat support because they can play with random people and communcate easily. but after a month on my server facebook group many user ask controll mapping to open the chat screen, on screen touch control to open the chat screen, make the chat button moveable like onscreen touch control.

so i add 2 more options to move the chat screen position and the chat button position on this commit , adenovan@1b9b61b , its only quick workaround i can implemented.

Android user playing with on screen touch control get upset when the chat button accidentally pressed when moving the analog stick (it's beside the analog on default setting). that 2 options solve their problem but far from ideal,moveable on screen touch button for chat button will be better. if you fine with that 2 options i will rebase it tommorow and make a new commit.

controll mapping, moveable chat button and the other user asked feature if they realy need it i think they can make a separate feature request and wait some good developer work on it. i can't fullfill their request because not having many knowledge and freetime to dig on onscreen touch control code and the control mapping code. so i left that another feature request to the dev here or some dev who want to work on it.

@tywald
Copy link
Contributor

tywald commented Nov 28, 2016

Is it possible to add/change when you press the shortcut CTRL+C to make you be able to chat right away?(right now you must click on the field that says "Chat Here" to be able to type) then if you press CTRL+C again the chat will close.

Just a suggestion based on making it a bit more convenient.

@adenovan
Copy link
Contributor Author

Its possible I think but I tried to code that and not found how to set the focus automatically to the text field.

The current Implementation can chat right away without using mouse. When the chat screen opened press right arrow key on keyboard or joy stick rigth so the focus is on textfield (one times only) . After that focus set you can open with ctrl + c and close with esc

@adenovan
Copy link
Contributor Author

adenovan commented Dec 5, 2016

rebased ,
can someone help me to set the focus to chatEdit_ when chatscreen ui pushed like tywald request?
tried many code and dig on viewgroup source but it still not working.

UpdateChat();
chatScreenVisible = true;
newChat = 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I tested a little and found a solution.
UI::EnableFocusMovement(true);
When I inserted this code into the last line of CreateViews(), focusing correctly.
I don't know this way is correct or not but it can fix the issue anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks xebra

@hrydgard hrydgard added this to the v1.5.0 milestone Mar 20, 2017
@hrydgard
Copy link
Owner

hrydgard commented Jun 6, 2017

What's the status of this, do you plan to do any more work on it? If it is to be merged it needs a rebase and some code review...

@adenovan
Copy link
Contributor Author

adenovan commented Jul 12, 2017

I have a plan to separate the opcode chat on the adhocserver before merging this one. Its must separated from the adhocserver thread and make its own thread.

If we spamming chat to the adhocserver it will affect the entire matchmaking library and it will cause compability problem in many adhoc games.

Its happened because the matching library is still busy processing chat request and not response matchmaking request in time.

@adenovan
Copy link
Contributor Author

adenovan commented Aug 8, 2017

the spamming chat problem is not occured when the proadhocserver serve less than 40 client. 40 client at once rarely happened on some vpn server used by user right now (Hamachi , Tunngle)

the buggy behavior occured when proadhocserver is under heavyload from 40 client or more and someone spam it. it caused by high latency too. actually separating opcode can be done later and if ppsspp have an official server we must separate the chat request to a new port and a new thread, we must create another game user node as p2p room and other chat processing separately from matching library there on the new port and thread not use the proadhocserver again.

so we can make 2 chat room without interrupting the matching process, 1 room for the matching peer and 1 room for global chat to let user meetup. and say hey someone i have this game lets play with me.

you need to review and test the code btw @hrydgard and check that new options also.

i have a plan to make a new UI which is only hide the inserttextbox when closed but still display the chat so user is not opening and closing chat again its like debug statistic and chat system used on many new games. (something like dota 2 chat) . it will start once i finish the tunneler because im more interested on developing that one than this feature.

@adenovan
Copy link
Contributor Author

adenovan commented Nov 10, 2017

looks like UI::TextEdit Cannot be used in qt ? it will be a bad experience chatting on linux or macxos platform. currently i try to make a gui for macos

never mind maybe i will just need to test it out , change nickname currently did not use it

@adenovan
Copy link
Contributor Author

adenovan commented Dec 12, 2017

did ppsspp have some implementation to handle shared mutex ?

i build a chat osm and refactor the chat code but its get deadlocked when OSM view access the chat data , looks like data race when reading the chat data at same time. i tried to mimic the current OSM implementation , but the difference is the data is read by 2 different UI and hide each other based on ChatScreen dialog flag or last update time flag. if i read only from Chat Screen dialog no deadlock occured.

ChatScreen and ChatOsm view need to read the data . while proadhoc thread and chat client thread supplied the data from the network. if i used condition variable when waiting the data it sleep the whole UI thread .

did the shared mutex really needed to solve this particular issues like https://stackoverflow.com/questions/19915152/c11-multiple-read-and-one-write-thread-mutex

this is the experimental chat commit adenovan@7fdb694

if it works good enough time to rebase this branch and merge the chat server

Edit
Nevermind looks like the deadlock occurred not by the mutex but due to inserting view on invisible UI

@hrydgard hrydgard modified the milestones: v1.6.0, v1.7.0 Mar 13, 2018
@hrydgard hrydgard modified the milestones: v1.7.0, v1.8.0 Sep 16, 2018
ChoiceWithValueDisplay *qc5 = networkingSettings->Add(new ChoiceWithValueDisplay(&g_Config.sQuickChat4, sy->T("Quick Chat 5"), nullptr));
qc5->OnClick.Handle(this, &GameSettingsScreen::OnChangeQuickChat4);
qc5->SetEnabledPtr(&g_Config.bEnableQuickChat);
#endif

Choose a reason for hiding this comment

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

Android studio produce an error: Cell to "ChoiceWithValueDisplay" is ambiguous.

I try code it in a more concise way
networkingSettings->Add(new ChoiceWithValueDisplay(&g_Config.sQuickChat0, sy->T("Quick Chat 1"), (const char *)nullptr))->OnClick.Handle(this, &GameSettingsScreen::OnChangeQuickChat0);
networkingSettings->Add(new ChoiceWithValueDisplay(&g_Config.sQuickChat1, sy->T("Quick Chat 2"), (const char *)nullptr))->OnClick.Handle(this, &GameSettingsScreen::OnChangeQuickChat1);
networkingSettings->Add(new ChoiceWithValueDisplay(&g_Config.sQuickChat2, sy->T("Quick Chat 3"), (const char *)nullptr))->OnClick.Handle(this, &GameSettingsScreen::OnChangeQuickChat2);
networkingSettings->Add(new ChoiceWithValueDisplay(&g_Config.sQuickChat3, sy->T("Quick Chat 4"), (const char *)nullptr))->OnClick.Handle(this, &GameSettingsScreen::OnChangeQuickChat3);
networkingSettings->Add(new ChoiceWithValueDisplay(&g_Config.sQuickChat4, sy->T("Quick Chat 5"), (const char *)nullptr))->OnClick.Handle(this, &GameSettingsScreen::OnChangeQuickChat4);

@jistjak
Copy link

jistjak commented Nov 12, 2018

is this still in the works? it'll be real cool if i can chat with my friends when they are somewhere else playing the same game. Also is there going to be voice chat? and is there going to be a blocklist or ban list for individuals to remove offending chatters? Also Friends' list, and maybe even sub-genre of friends' lists (like RPG friends, Racing friends, etc.)? Could i map the chat button to the / (slash button, aka non-shifted '?' question mark.)

@adenovan
Copy link
Contributor Author

All of that youre asking is possible , yes its still work in progress

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