0ad/0ad
0ad/0ad
4
5
Fork 14
Code Issues 1.5k Pull Requests 29 Actions Projects Releases Wiki Activity

Don't use std::shared_ptr in CStrIntern #6965

Merged
phosit merged 1 commits from phosit/0ad:Don't_use_shared_ptr_in_CStdIntern into main 2024-08-25 11:58:27 +02:00
Conversation 5 Commits 1 Files Changed 1 +15 -14
phosit commented 2024-08-24 12:21:53 +02:00
Member
Copy Link

Now there is less allocation and reference counting.

Continuation of https://code.wildfiregames.com/D5155

Now there is less allocation and reference counting. Continuation of https://code.wildfiregames.com/D5155
phosit added 1 commit 2024-08-24 12:21:56 +02:00
Imported from Phab
Some checks reported warnings
0ad-freebsd/pipeline/pr-main This commit is unstable
Details
0ad-windows/pipeline/pr-main This commit is unstable
Details
0ad-checkrefs/pipeline/pr-main This commit looks good
Details
0ad-macos/pipeline/pr-main This commit is unstable
Details
0ad-linux/pipeline/pr-main This commit looks good
Details
8d48b161ee
sera commented 2024-08-24 12:25:23 +02:00
Member
Copy Link

A commit message "Imported from Phab" is no good :)

A commit message "Imported from Phab" is no good :)
phosit commented 2024-08-24 12:31:14 +02:00
Author
Member
Copy Link

A commit message "Imported from Phab" is no good :)

Why?
Should it be "Don't use std::shared_ptr in CStrIntern" again?
Normaly a branch starts as "Initial commit". That isn't much better.

> A commit message "Imported from Phab" is no good :) Why? Should it be "Don't use std::shared_ptr in CStrIntern" again? Normaly a branch starts as "Initial commit". That isn't much better.
sera commented 2024-08-24 12:37:10 +02:00
Member
Copy Link

The branch name and the title and message of the pull request are irrelevant for how the commit will look like once merge.

You can fix the commit message with "git commit --amend", then check how it looks like with "git log".

The branch name and the title and message of the pull request are irrelevant for how the commit will look like once merge. You can fix the commit message with "git commit --amend", then check how it looks like with "git log".
Stan commented 2024-08-24 12:37:24 +02:00
Owner
Copy Link

With the rebase workflow "Imported from Phab" will be committed to main. There is no squash. Nor commit renaming. You have to do that on the branch.

With the rebase workflow "Imported from Phab" will be committed to main. There is no squash. Nor commit renaming. You have to do that on the branch.
phosit commented 2024-08-24 12:59:10 +02:00
Author
Member
Copy Link

Thanks for the explanation

Thanks for the explanation
phosit force-pushed Don't_use_shared_ptr_in_CStdIntern from 8d48b161ee to 89750f84e6 2024-08-24 14:32:55 +02:00 Compare
sera reviewed 2024-08-24 14:52:59 +02:00
source/ps/CStrIntern.cpp Outdated
@ -108,3 +111,1 @@
std::shared_ptr<CStrInternInternals> internals = std::make_shared<CStrInternInternals>(str, len);
g_Strings.insert(std::make_pair(internals->data, internals));
return internals.get();
return &g_Strings.insert({str, {str, len}}).first->second;
sera commented 2024-08-24 14:52:50 +02:00
Member
Copy Link

maybe make the return type a reference to signal doesn't need delete and is never null?

maybe make the return type a reference to signal doesn't need delete and is never null?
phosit commented 2024-08-24 16:30:09 +02:00
Author
Member
Copy Link

I've done that in https://code.wildfiregames.com/D5157

I've done that in https://code.wildfiregames.com/D5157
sera commented 2024-08-24 16:36:05 +02:00
Member
Copy Link

This patch here returns a pointer tho

This patch here returns a pointer tho
sera commented 2024-08-24 16:37:27 +02:00
Member
Copy Link

I see, someone wasn't a big fan of it :)

I see, someone wasn't a big fan of it :)
phosit force-pushed Don't_use_shared_ptr_in_CStdIntern from 89750f84e6 to 318c0aecd2 2024-08-24 16:31:53 +02:00 Compare
phosit force-pushed Don't_use_shared_ptr_in_CStdIntern from 318c0aecd2 to 45d51b7cf6 2024-08-24 16:38:21 +02:00 Compare
phosit added the
Theme
Core engine
 label 2024-08-24 17:07:38 +02:00
sera approved these changes 2024-08-24 20:26:46 +02:00
phosit merged commit 62c0080e1b into main 2024-08-25 11:58:27 +02:00
phosit deleted branch Don't_use_shared_ptr_in_CStdIntern 2024-08-25 11:58:27 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
sera
Labels
Clear labels   
Closed
Duplicate

  
Closed
Fixed

  
Closed
Invalid

  
Closed
Needs info

  
Closed
Won't fix

  
Closed
Works for me

  
Difficulty
Hard

  
Difficulty
Medium

  
Difficulty
Simple

  
Needed for Beta

   
Needs Design Input

   
Needs Info

   
Pathfinding
  
Priority
1: Release Blocker

  
Priority
2: Must Have

  
Priority
3: Should Have

  
Priority
4: Nice To Have

  
Priority
5: If Time Permits

  
Regression
  
Theme
AI

  
Theme
Art & Animation

  
Theme
Atlas editor

  
Theme
Build & Packages

  
Theme
Core engine

  
Theme
Internationalization & Localization

  
Theme
Maps

  
Theme
Multiplayer Lobby

  
Theme
Music & Sound FX

  
Theme
Network

  
Theme
Non-game systems

  
Theme
Simulation

  
Theme
UI & Simulation

  
Theme
UI – Game setup

  
Theme
UI – In-game

  
Theme
UI – Miscellaneous

  
Theme
Website & Forum

  
Type
Defect

  
Type
Enhancement

  
Type
Task

No Label
Closed
Duplicate
Closed
Fixed
Closed
Invalid
Closed
Needs info
Closed
Won't fix
Closed
Works for me
Difficulty
Hard
Difficulty
Medium
Difficulty
Simple
Needed for Beta
Needs Design Input
Needs Info
Pathfinding
Priority
1: Release Blocker
Priority
2: Must Have
Priority
3: Should Have
Priority
4: Nice To Have
Priority
5: If Time Permits
Regression
Theme
AI
Theme
Art & Animation
Theme
Atlas editor
Theme
Build & Packages
Theme
Core engine
Theme
Internationalization & Localization
Theme
Maps
Theme
Multiplayer Lobby
Theme
Music & Sound FX
Theme
Network
Theme
Non-game systems
Theme
Simulation
Theme
UI & Simulation
Theme
UI – Game setup
Theme
UI – In-game
Theme
UI – Miscellaneous
Theme
Website & Forum
Type
Defect
Type
Enhancement
Type
Task
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: 0ad/0ad#6965
No description provided.
Delete Branch "phosit/0ad:Don't_use_shared_ptr_in_CStdIntern"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?