chrononeko / bugtracker

Chrononeko Bugtracker
0 stars 0 forks source link

feat: sendMsg return msgId #4

Closed Hieuzest closed 1 year ago

Hieuzest commented 1 year ago

are your guys kiding on return sendMsg result?that does not make any sense.

I think we could compare message contents between sent mesaages and records from msgListInfo, but that would be dirty and may mismatch in several situations. I recognise that people can not distinguish similar message either, so this should be fine.

another solution might be directly match from sending timestamps and client sequences. Still there could be issues.

BTW, is it even possible to pass given args to reflect on msgList? Seems there is no shortcut

std-microblock commented 1 year ago

I think we could compare message contents between sent mesaages and records from msgListInfo, but that would be dirty...

I haven't checked the return value of sendMsg result, and that's why I didn't flag the commit as close the issue.

If there's not any useful result in sendMsg return value, there's a IPC send callback after sending the message that could be used.

Hieuzest commented 1 year ago

that's why i mention matching, cuz i don't find any native way to matching from sendMsg call and its corresponding callback.

Currently i have implemented it, through adding a dummy message element. But there are lots of things to be decided, such as:

Hieuzest commented 1 year ago

d0c5bf24 i don't feel i should push this upstream though

std-microblock commented 1 year ago

I don't think the callback you're using is the callback I mentioned before. I'm waiting for @ilharp 's debugger to take a look at it.

std-microblock commented 1 year ago

is adding a dummy element really a good idea? note that it could be sent to the server and any malformed packet may probably seen as proof of using a bot framework. We should avoid this

std-microblock commented 1 year ago

If there's no beautiful solution from cpp, I can also resolve it from the native side, but we'll need a .node native module which can be hard to maintain.

Hieuzest commented 1 year ago
  • to what extend can we modify it, as to preserve the purity of a hook framework

just as i said. i agree so i'm waiting for others to find a more elegent way to do this.

std-microblock commented 1 year ago

However, I believe that there should be a way on the JS side to match the send success callback with the send message invocation, as the QQNT itself would display a loading icon beside the message until the message was successfully sent.

std-microblock commented 1 year ago

Let's wait for the debugger first.

Hieuzest commented 1 year ago

However, I believe that there should be a way on the JS side to match the send success callback with the send message invocation, as the QQNT itself would display a loading icon beside the message until the message was successfully sent.

Actually, it don't need to, since there do exist a list holding all pending messages and tracking status of each. NT client has nothing to do with the invovations but simply present these messages so there is no matching.

std-microblock commented 1 year ago

Actually, it don't need to, since there do exist a list holding all pending messages and tracking status of each. NT client has nothing to do with the invovations but simply present these messages so there is no matching.

Nah, have you seen the loading icon? How could it display a loading icon without matching it? Also if you send some sexy images or are disconnected from the network, the message will fail to send, and how could it display the small retry button beside the corresponding message if not matching it?

Hieuzest commented 1 year ago

Think about what matching here are we talked about. sendMsg pushes the message into pending list, in where it is already fullfilled with msgId and other properties. the invocation is gone. What NT client got is a fullfilled message with possibily unstable sendStatus. The case is, when we send massive messages at once, we will be unable to distinguish from them

std-microblock commented 1 year ago

oh, I see

std-microblock commented 1 year ago

Let's also maintain a list which contains the messages that are not received yet.

std-microblock commented 1 year ago
[ 🔗 ↑ ] CmdName: nodeIKernelMsgListener/onAddSendMsg Payload: {
  msgRecord: {
    msgId: '7272438935119848258',
    msgRandom: '1567328768',
    msgSeq: '285',
    cntSeq: '0',
    chatType: 2,
    msgType: 2,
    subMsgType: 1,
    sendType: 1,
    senderUid: 'u_OtJTqpFvVc3bA--L0ahAPA',
    peerUid: '892539614',
    channelId: '',
    guildId: '',
    guildCode: '0',
    fromUid: '0',
    fromAppid: '0',
    msgTime: '1693246638',
    msgMeta: '0x',
    sendStatus: 1,
    sendMemberName: '',
    sendNickName: '时空猫猫',
    guildName: '',
    channelName: '',
    elements: [ [Object] ],
    records: [],
    emojiLikesList: [],
    commentCnt: '0',
    directMsgFlag: 0,
    directMsgMembers: [],
    peerName: 'MicroBlock1、时空猫猫',
    freqLimitInfo: null,
    editable: true,
    avatarMeta: '',
    avatarPendant: '',
    feedId: '',
    roleId: '0',
    timeStamp: '0',
    clientIdentityInfo: null,
    isImportMsg: false,
    atType: 0,
    roleType: 0,
    fromChannelRoleInfo: { roleId: '0', name: '', color: 0 },
    fromGuildRoleInfo: { roleId: '0', name: '', color: 0 },
    levelRoleInfo: { roleId: '0', name: '', color: 0 },
    recallTime: '0',
    isOnlineMsg: false,
    generalFlags: '0x',
    clientSeq: '28763',
    fileGroupSize: null,
    foldingInfo: null,
    nameType: 0,
    avatarFlag: 0,
    anonymousExtInfo: null,
    personalMedal: null,
    roleManagementTag: null
  }
} Full: [
  'IPC_DOWN_2',
  { type: 'request', eventName: 'ns-ntApi-2' },
  [
    {
      cmdName: 'nodeIKernelMsgListener/onAddSendMsg',
      cmdType: 'event',
      payload: [Object]
    }
  ]
]

Here's a clientSeq field in this callback, and this can probably indicate the order of the messages that were sent if we are able to get the current clientSeq number.

std-microblock commented 1 year ago

Okay, currently, as I observed, the clientSeq wouldn't change without sending a message. This would enable us to correspond the message object with the send request

Hieuzest commented 1 year ago

i can confirm this indicate the order, but i don't think ipc can preserve request order

std-microblock commented 1 year ago

i can confirm this indicate the order, but i don't think ipc can preserve request order

JS is single-threaded, which means it won't go any further before successfully invoking the IPC call. We could have a try at least.

std-microblock commented 1 year ago

Note that it's different in different peers.

Hieuzest commented 1 year ago

i recognize that it is impossible to do a benchmark of sending really massive messages. Maybe i'm overthinking about it. the ipc is fast enough that it is totally afforable if we add some soft restriction like rate limit of 20 messages/s

std-microblock commented 1 year ago

i recognize that it is impossible to do a benchmark of sending really massive messages. Maybe i'm overthinking about it. the ipc is fast enough that it is totally afforable if we add some soft restriction like rate limit of 20 messages/s

the bottleneck of sending message is not at IPC, it's at network.

Even a rate limit of 2msg/s is totally acceptable for most situations